Skip to content

Low cardinality key for uri is reported as UNKNOWN in DefaultServerRequestObservationConvention #40923

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

Closed
nodshams opened this issue May 28, 2024 · 4 comments
Labels
for: external-project For an external project and not something we can fix status: duplicate A duplicate of another issue

Comments

@nodshams
Copy link

Hello community 👋,

We have a test that validates the http.server.requests metric with tags collected in Micrometer in spring-graphql application as follows:

assertThat(meterRegistry
                        .timer(
                                "http.server.requests",
                                Tags.of(
                                        "error", "none",
                                        "exception", "none",
                                        "method", "POST",
                                        "outcome", "SUCCESS",
                                        "status", "200",
                                        "uri", "/graphql",
                                        "user", warmupUserName))
                        .count())
                .isEqualTo(400);

Issue

After upgrading to Spring Boot version 3.3.0, this test is failing. Upon debugging, I discovered that the low-cardinality key uri is now being reported as UNKNOWN in DefaultServerRequestObservationConvention.

Since spring boot 3.3.0 includes dependency update for graphql-java v22.0 can you help me to find the root cause of the uri being collected as UNKNOWN

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 28, 2024
@bclozel
Copy link
Member

bclozel commented May 28, 2024

Can you share a minimal sample we can have a look at?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label May 28, 2024
@nodshams
Copy link
Author

nodshams commented May 28, 2024

Hey @bclozel,

test is quite simple. We have DataDog dashboards that expect certain tags in the http.server.requests metric reported by our application. To ensure these metrics are reported correctly with all necessary tags, we have a simple test.

it's not visible here but before the test starts, we run a warmup client that sends 400 requests to the /graphql endpoint (localhost:8080/graphql) on onApplicationEvent to warm up the application.

@Test
public void it_pongs() {
    RestTemplate restTemplate = builder.build();

    assertThat(restTemplate
                    .getForEntity("http://localhost:53609/ping", Void.class)
                    .getStatusCode())
            .isEqualTo(HttpStatus.OK);

    assertThat(meterRegistry
                    .timer(
                            "http.server.requests",
                            Tags.of(
                                    "error", "none",
                                    "exception", "none",
                                    "method", "POST",
                                    "outcome", "SUCCESS",
                                    "status", "200",
                                    "uri", "/graphql",
                                    "user", warmupUserName))
                    .count())
            .isEqualTo(400);
}

Issue

After upgrading to Spring Boot version 3.3.0, this test is now failing. The uri tag is being reported as UNKNOWN instead of /graphql.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 28, 2024
@nodshams
Copy link
Author

forgot to mention, we have ExtendedServerRequestObservationConvention to collect customs tags for http.server.requests

public class ExtendedServerRequestObservationConvention extends DefaultServerRequestObservationConvention {

    @NotNull @Override
    public KeyValues getLowCardinalityKeyValues(ServerRequestObservationContext context) {
        return super.getLowCardinalityKeyValues(context).and(custom(context));
    }

    @NotNull @Override
    public KeyValues getHighCardinalityKeyValues(ServerRequestObservationContext context) {
        return super.getHighCardinalityKeyValues(context).and(custom(context));
    }

    protected KeyValue custom(ServerRequestObservationContext context) {
        return KeyValue.of(
                "user",
                SecurityHelper.extractUserNameFromAuthHeaderOrDefault(
                        context.getCarrier().getHeader(HttpHeaders.AUTHORIZATION)));
    }
}

@bclozel
Copy link
Member

bclozel commented May 28, 2024

I don't think anything there is relevant to the issue, this information is missing out of the box at runtime, even without custom conventions. I've created spring-projects/spring-graphql#987 to track this.

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2024
@bclozel bclozel added status: duplicate A duplicate of another issue for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants