Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command name to count metrics #437

Merged
merged 16 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.JsonNode;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -184,15 +183,6 @@ public record Error(
"Error fields can not contain the reserved message key.");
}
}

/**
* Constructor that sets documents only the message.
*
* @param message Error message.
*/
public Error(String message) {
this(message, Collections.emptyMap(), Response.Status.OK);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import io.stargate.sgv2.api.common.security.challenge.ChallengeSender;
import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
import io.vertx.ext.web.RoutingContext;
import java.util.Collections;
import java.util.List;
import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -34,12 +36,12 @@ public ErrorChallengeSender(
String headerName,
ObjectMapper objectMapper) {
this.objectMapper = objectMapper;

// create the response
String message =
"Role unauthorized for operation: Missing token, expecting one in the %s header."
.formatted(headerName);
CommandResult.Error error = new CommandResult.Error(message);
CommandResult.Error error =
new CommandResult.Error(message, Collections.emptyMap(), Response.Status.UNAUTHORIZED);
commandResult = new CommandResult(List.of(error));
}

Expand All @@ -59,8 +61,8 @@ public Uni<Boolean> apply(RoutingContext context, ChallengeData challengeData) {
.headers()
.set(HttpHeaders.CONTENT_LENGTH, String.valueOf(response.getBytes().length));

// always set status to 200
context.response().setStatusCode(200);
// Return the status code from the challenge data
context.response().setStatusCode(challengeData.status);

// write and map to true
return Uni.createFrom()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.stargate.sgv2.jsonapi.api.v1.metrics;

import io.micrometer.core.instrument.Tag;

public class MetricsConstants {

// same as V1 io.stargate.core.metrics.StargateMetricConstants#UNKNOWN
public static final String UNKNOWN_VALUE = "unknown";

public static final Tag ERROR_TRUE_TAG = Tag.of("error", "true");

public static final Tag ERROR_FALSE_TAG = Tag.of("error", "false");

public static final Tag MODULE_TAG = Tag.of("module", "jsonapi");
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved

public static final String ERROR_CLASS = "errorClass";

public static final String ERROR_CODE = "errorCode";
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved

public static final String TENANT = "tenant";
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved

public static final String COMMAND = "command";

public static final String NA = "NA";

public static final Tag DEFAULT_ERROR_CLASS_TAG = Tag.of(ERROR_CLASS, NA);

public static final Tag DEFAULT_ERROR_CODE_TAG = Tag.of(ERROR_CODE, NA);

public static final String COUNT_METRICS_NAME = "command_processor_count";
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved

public static final String TIMER_METRICS_NAME = "command_processor_total";
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package io.stargate.sgv2.jsonapi.service.processor;

import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Tags;
import io.smallrye.mutiny.Uni;
import io.stargate.sgv2.api.common.StargateRequestInfo;
import io.stargate.sgv2.jsonapi.api.model.command.Command;
import io.stargate.sgv2.jsonapi.api.model.command.CommandContext;
import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
import io.stargate.sgv2.jsonapi.api.v1.metrics.MetricsConstants;
import io.stargate.sgv2.jsonapi.exception.JsonApiException;
import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableCommandResultSupplier;
import io.stargate.sgv2.jsonapi.service.bridge.executor.QueryExecutor;
Expand All @@ -25,15 +30,24 @@
@ApplicationScoped
public class CommandProcessor {

private final MeterRegistry meterRegistry;

private final QueryExecutor queryExecutor;

private final CommandResolverService commandResolverService;

private final StargateRequestInfo stargateRequestInfo;

@Inject
public CommandProcessor(
QueryExecutor queryExecutor, CommandResolverService commandResolverService) {
MeterRegistry meterRegistry,
QueryExecutor queryExecutor,
CommandResolverService commandResolverService,
StargateRequestInfo stargateRequestInfo) {
this.meterRegistry = meterRegistry;
this.queryExecutor = queryExecutor;
this.commandResolverService = commandResolverService;
this.stargateRequestInfo = stargateRequestInfo;
}

/**
Expand All @@ -46,36 +60,92 @@ public CommandProcessor(
*/
public <T extends Command> Uni<CommandResult> processCommand(
CommandContext commandContext, T command) {
// start by resolving the command, get resolver
return commandResolverService
.resolverForCommand(command)

// resolver can be null, not handled in CommandResolverService for now
.flatMap(
resolver -> {
// if we have resolver, resolve operation and execute
Operation operation = resolver.resolveCommand(commandContext, command);
return operation.execute(queryExecutor);
})
final Tag commandTag = Tag.of(MetricsConstants.COMMAND, command.getClass().getSimpleName());
final String tenant =
stargateRequestInfo != null && stargateRequestInfo.getTenantId().isPresent()
? stargateRequestInfo.getTenantId().get()
: MetricsConstants.UNKNOWN_VALUE;
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
final Tag tenantTag = Tag.of(MetricsConstants.TENANT, tenant);
final long start = System.currentTimeMillis();
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
return Uni.createFrom()
.item(start)
.onItem()
.transformToUni(
startTime -> {
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
// start by resolving the command, get resolver
return commandResolverService
.resolverForCommand(command)

// handler failures here
.onFailure()
.recoverWithItem(
t -> {
// DocsException is supplier of the CommandResult
// so simply return
if (t instanceof JsonApiException jsonApiException) {
return jsonApiException;
}
// resolver can be null, not handled in CommandResolverService for now
.flatMap(
resolver -> {
// if we have resolver, resolve operation and execute
Operation operation = resolver.resolveCommand(commandContext, command);
return operation.execute(queryExecutor);
})

// otherwise use generic for now
return new ThrowableCommandResultSupplier(t);
})
// handler failures here
.onItemOrFailure()
.transform(
(item, failure) -> {
if (failure != null) {
if (failure instanceof JsonApiException jsonApiException) {
return jsonApiException;
}
return new ThrowableCommandResultSupplier(failure);

// if we have a non-null item
// call supplier get to map to the command result
.onItem()
.ifNotNull()
.transform(Supplier::get);
} else {
// return the tags and command result
return item;
}
})
.onItem()
.ifNotNull()
.transform(Supplier::get)
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
.onItem()
.transform(
result -> {
Tag errorTag = MetricsConstants.ERROR_FALSE_TAG;
;
Tag errorClassTag = MetricsConstants.DEFAULT_ERROR_CLASS_TAG;
Tag errorCodeTag = MetricsConstants.DEFAULT_ERROR_CODE_TAG;
if (null != result.errors() && !result.errors().isEmpty()) {
errorTag = MetricsConstants.ERROR_TRUE_TAG;
String errorClass =
(String)
result
.errors()
.get(0)
.fields()
.getOrDefault(
MetricsConstants.ERROR_CLASS,
MetricsConstants.UNKNOWN_VALUE);
errorClassTag = Tag.of(MetricsConstants.ERROR_CLASS, errorClass);
String errorCode =
(String)
result
.errors()
.get(0)
.fields()
.getOrDefault(
MetricsConstants.ERROR_CODE,
MetricsConstants.UNKNOWN_VALUE);
errorCodeTag = Tag.of(MetricsConstants.ERROR_CODE, errorCode);
}
Tags tags =
Tags.of(commandTag, tenantTag, errorTag, errorClassTag, errorCodeTag);
// add metrics
meterRegistry
.counter(MetricsConstants.COUNT_METRICS_NAME, tags)
.increment();
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
meterRegistry
.timer(MetricsConstants.TIMER_METRICS_NAME, tags)
.record(
System.currentTimeMillis() - start,
java.util.concurrent.TimeUnit.MILLISECONDS);
// return the command result
return result;
});
});
maheshrajamani marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@
import io.restassured.http.ContentType;
import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile;
import io.stargate.sgv2.jsonapi.api.v1.CollectionResource;
import io.stargate.sgv2.jsonapi.api.v1.NamespaceResource;
import io.stargate.sgv2.jsonapi.api.v1.GeneralResource;
import java.util.List;
import org.junit.jupiter.api.Test;

@QuarkusTest
@TestProfile(NoGlobalResourcesTestProfile.Impl.class)
public class UnauthorizedMetricsTest {
public class MetricsTest {

@Test
public void namespaceResource() {
public void unauthorizedNamespaceResource() {
String json =
"""
{
Expand All @@ -32,14 +32,13 @@ public void namespaceResource() {
.contentType(ContentType.JSON)
.body(json)
.when()
.post(NamespaceResource.BASE_PATH, "keyspace")
.post(GeneralResource.BASE_PATH)
.then()
.statusCode(200);
.statusCode(401);

String metrics = given().when().get("/metrics").then().statusCode(200).extract().asString();
List<String> httpMetrics =
metrics.lines().filter(line -> line.startsWith("http_server_requests_seconds")).toList();

assertThat(httpMetrics)
.allSatisfy(
line ->
Expand All @@ -51,7 +50,7 @@ public void namespaceResource() {
}

@Test
public void collectionResource() {
public void unauthorizedCollectionResource() {
String json = """
{
"find": {
Expand All @@ -66,7 +65,41 @@ public void collectionResource() {
.when()
.post(CollectionResource.BASE_PATH, "keyspace", "collection")
.then()
.statusCode(200);
.statusCode(401);

String metrics = given().when().get("/metrics").then().statusCode(200).extract().asString();
List<String> httpMetrics =
metrics.lines().filter(line -> line.startsWith("http_server_requests_seconds")).toList();

assertThat(httpMetrics)
.allSatisfy(
line ->
assertThat(line)
.containsAnyOf(
"uri=\"/v1\"",
"uri=\"/v1/{namespace}\"",
"uri=\"/v1/{namespace}/{collection}\""));
}

@Test
public void unauthorizedGeneralResource() {
String json =
"""
{
"createNamespace": {
"name": "purchase_database"
}
}
""";

// ensure namespace not in tags when no auth token used
given()
.contentType(ContentType.JSON)
.body(json)
.when()
.post(GeneralResource.BASE_PATH)
.then()
.statusCode(401);

String metrics = given().when().get("/metrics").then().statusCode(200).extract().asString();
List<String> httpMetrics =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.quarkus.test.junit.TestProfile;
import io.stargate.sgv2.common.testprofiles.NoGlobalResourcesTestProfile;
import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
import java.util.Collections;
import java.util.Map;
import javax.inject.Inject;
import javax.ws.rs.core.Response;
Expand Down Expand Up @@ -37,7 +38,8 @@ public void happyPath() throws Exception {

@Test
public void withoutProps() throws Exception {
CommandResult.Error error = new CommandResult.Error("My message.");
CommandResult.Error error =
new CommandResult.Error("My message.", Collections.emptyMap(), Response.Status.OK);

String result = objectMapper.writeValueAsString(error);

Expand Down
Loading