OpenFeign - Aggregateable http.client URLs for discovery

[original-codematrix]

Problem Statement

The main problem that I'm currently facing is that I have path variables in the URL that look like that: Base path: http://someurl:someport Method:

@RequestLine("GET /api/v1/someapi/{uuid}/something")
SomeObject getSomething(@Param("uuid") UUID uuid);

When we now execute a feign call inside of a Transaction from a REST-Endpoint in spring-boot I'll get something like that in the Span: GET http://someurl:someport/api/v1/someapi/811e51ca-1fda-4d38-be36-f128b286dc61 /something as URL for the http.client.

To be able to aggregate in the discovery view in the UI I would need to have the placeholders in the URLs instead of the actual values to aggregate over the specific calls for the endpoint to be able to know the average response times of that.URL

Solution Brainstorm

The template URL/URI should be used instead of the actual request URL/URI. The actual values should be passed somehow else. That is already implemented in the MicrometerCapability which I use currently for Grafana where just the placeholders are set, so we can aggregate over that to know the average count of calls we made to a specific endpoint.

Maybe a toggle for that feature would be nice for some other people. IMO it makes sense to have this to be able to aggregate over the URLs/URIs because, with its current implementation (for OpenFeign), I'm not able to do that.

[adinauer]

Hello @original-codematrix, you could use SentryFeignClient.BeforeSpanCallback to modify the span before it is sent to Sentry. It could look something like the following - just make sure to guard the code against eventual exceptions / nulls / etc.:

        Feign.builder()
            .addCapability(
                new SentryCapability(
                    (span, request, response) -> {
                      RequestTemplate feignTemplate = request.requestTemplate().methodMetadata().template();
                      String method = feignTemplate.method();
                      String url = feignTemplate.url();
                      span.setDescription(method + " " + url);

                      return span;
                    }))

If you need more context around these lines, please check out our sample here. I've simply modified the beforeSpanCallback. Here's also some (light) docs on the matter: https://docs.sentry.io/platforms/java/guides/spring-boot/performance/instrumentation/open-feign/#modify-or-drop-spans

which produces spans like this:

        {
            "start_timestamp": 1681794046.194695,
            "timestamp": 1681794046.219012,
            "trace_id": "e1d15c7f73c8487aae1ff043bd95373c",
            "span_id": "c7131bd5ebf346ab",
            "parent_span_id": "771645eb74474bff",
            "op": "http.client",
            "description": "GET /todos/{id}",
            "status": "ok",
            "data": {}
        }

Hope this helps. I'll talk to the team whether this is something we want to do in the SDK and if so in what way. Will update here once we know more.

[adinauer]

We've just discussed and this might be a breaking change for others that don't expect their span descriptions to change. We may revisit this in the future when we change some of the performance product around spans and transactions.

As a workaround people who want the template can use the snippet above.

[original-codematrix]

@adinauer thx for the response! Yeah, I already thought this would be the reason for not implementing that change.