getsentry / sentry-java

A Sentry SDK for Java, Android and other JVM languages.
https://docs.sentry.io/
MIT License
1.16k stars 435 forks source link

OpenTelemetry spans appearing with `op` `http` missing `.client` / `.server` #3904

Open adinauer opened 3 days ago

adinauer commented 3 days ago

Description

Some product features rely on span ops being a specific value, for example http.client or http.server to distinguish client from server HTTP requests, to popular inbound and outbound requests.

Features related to other insights in Sentry product, also rely on other span.ops as well. list of span operations

In hybrid Java setups with Sentry <> OpenTelemetry, this span op is not most likely cause for this is manual instrumentation where OpenTelemetry SpanKind is not set to either SERVER or CLIENT.

Possible solutions:

  1. Document how to make use of APIs

Document how (and why) to set op for HTTP requests and how to set SpanKind when using OpenTelemetry API.

  1. Expose SpanKind in Sentry API

This does still requires us to document how customers should use the API to get the most out of Sentry. It doesn't just work automatically. It does help not relying on the spans op.

  1. Detect client vs. server in SDK

We could offer some config option to set the span kind based on the instrumentation name. However, OpenTelemetry auto instrumentation should already be setting span kind correctly anyways and this seems more complex than simply setting the span kind on spans directly.

Manual instrumentation would then have to use separate instrumentation names for different things being instrumented.

  1. Detect client vs. server in relay

It could set span kind based on lists of known instrumentations, e.g. RestClient is always a client span. However this shouldn't be a problem as the OpenTelemetry always sets span kind as far as we know at the moment.

Not sure how that would work for spans created via manual instrumentation.

  1. Do not override the op in SpanDescriptionExtractor and rely on op being set on the span correctly 5a. Based on instrumentation name

How do we know which span we should generate op for and which we shouldn't? Can we use instrumentation name to only extract for spans coming from OpenTelemetry SDK auto instrumentation?

Is this even better than what's going on now? Instead of asking customers to set the SpanKind, we're expecting them to set op according to rules they need to follow.

5b. Based on SpanKind

We could also look at SpanKind and only generate op automatically, if it's something other than INTERNAL.

Concerns

  1. the sentry product relies on span operations which is not a standard of OTel (similar discussion here)
adinauer commented 3 days ago

I just tested this using v8 and if SpanKind isn't set, it's hard to get this right.

Here's the code I used for testing:

Span span = tracer.spanBuilder("http.client1")
  .setAttribute(HTTP_REQUEST_METHOD, "GET")
  .startSpan();

try (final @NotNull Scope spanScope = span.makeCurrent()) {
  ISpan currentSpan = Sentry.getSpan();
  currentSpan.setOperation("http.client2");
  ...

and this is the JSON that was produced:

"spans":
[
    {
        "start_timestamp": 1732020576.835432,
        "timestamp": 1732020576.857683,
        "trace_id": "f9118105af4a2d42b4124532cd780521",
        "span_id": "ef477b3dd0e72174",
        "parent_span_id": "64958862e9dd3a0b",
        "op": "http",
        "description": "http.client2",
        "status": "ok",
        "origin": "auto.opentelemetry",
        "data":
        {
            "otel.instrumentation.name": "tracerForSpringBootDemo",
            "http.request.method": "GET",
            "otel.kind": "INTERNAL",
            "thread.name": "http-nio-8080-exec-1",
            "thread.id": 68
        }
    },

Note that op has become http because SpanDescriptionExtractor kicks in and overrides the op that was set originally due to HTTP attributes being present.

When the code sample also sets SpanKind:

Span span = tracer.spanBuilder("http.client1")
  .setAttribute(HTTP_REQUEST_METHOD, "GET")
  .setSpanKind(SpanKind.SERVER)
  .startSpan();
try (final @NotNull Scope spanScope = span.makeCurrent()) {
  ISpan currentSpan = Sentry.getSpan();
  currentSpan.setOperation("http.client2");

it does produce the expected op that makes the product features work:

"spans":
[
    {
        "start_timestamp": 1732020659.685268,
        "timestamp": 1732020659.706092,
        "trace_id": "f9118105af4a2d42b4124532cd750077",
        "span_id": "fa331872eb1adcd0",
        "parent_span_id": "588a33757a7fb622",
        "op": "http.server",
        "description": "http.client2",
        "status": "ok",
        "origin": "auto.opentelemetry",
        "data":
        {
            "otel.instrumentation.name": "tracerForSpringBootDemo",
            "http.request.method": "GET",
            "otel.kind": "SERVER",
            "thread.name": "http-nio-8080-exec-2",
            "thread.id": 70
        }
    },
adinauer commented 1 day ago

Our current plan is to improve docs around setting SpanKind. We've also merged a fix for not being able to set op manually using v8 if there's certain span attributes present.