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

Allow overwriting/modyfing span resource in SpanProcessor #5395

Open
fprochazka opened this issue Apr 23, 2023 · 6 comments
Open

Allow overwriting/modyfing span resource in SpanProcessor #5395

fprochazka opened this issue Apr 23, 2023 · 6 comments
Labels
blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project

Comments

@fprochazka
Copy link

Is your feature request related to a problem? Please describe.

I'm using OTEL with DataDog (DD), and it would greatly help visibility if I could overwrite the service name for some operations (spans).

DD doesn't properly understand the semantics of OTEL, and the problem is extra visible when some services are instrumented using OTEL, and some are instrumented using the native DD agent. DD creates a separate "service" for database calls, HTTP calls, etc - basically, everything that "leaves" the service and that's super useful, because then the time spent in DB or HTTP calls really pops out and becomes super visible.

Describe the solution you'd like

Basically I'd like to be able to modify the resource of a span. Right now the resource field is final and ReadWriteSpan cannot modify it.

Describe alternatives you've considered

Currently, I'm experimenting with hacking it using reflection, but I would like to avoid having to use that in the PROD... but as a prototype, it works really well...

OverrideServiceNameSpanProcessor
public class OverrideServiceNameSpanProcessor implements SpanProcessor
{

    private static final Class<?> SDK_SPAN_CLASS;
    private static final Field SDK_SPAN_RESOURCE_FIELD;

    static {
        try {
            SDK_SPAN_CLASS = Class.forName("io.opentelemetry.sdk.trace.SdkSpan");

            SDK_SPAN_RESOURCE_FIELD = SDK_SPAN_CLASS.getDeclaredField("resource");
            SDK_SPAN_RESOURCE_FIELD.setAccessible(true);

        } catch (ClassNotFoundException | NoSuchFieldException e) {
            throw new IllegalStateException(e);
        }
    }

    private static final Map<String, String> SCOPE_TO_SERVICE_NAME_SUFFIX = Map.ofEntries(
        Map.entry("io.opentelemetry.jdbc", "jdbc"),
        Map.entry("io.opentelemetry.apache-httpclient-5.0", "httpclient"),
        Map.entry("io.opentelemetry.http-url-connection", "httpclient"),
        Map.entry("io.opentelemetry.aws-sdk-1.11", "aws")
    );

    @Override
    public void onStart(final Context parentContext, final ReadWriteSpan span)
    {
        if (!SDK_SPAN_CLASS.isInstance(span)) {
            return; // noop
        }

        InstrumentationScopeInfo instrumentationInfo = span.getInstrumentationScopeInfo();
        @Nullable String resourceServiceNameSuffix = SCOPE_TO_SERVICE_NAME_SUFFIX.get(instrumentationInfo.getName());
        if (resourceServiceNameSuffix == null) {
            return; // noop
        }

        try {
            var resource = (Resource) SDK_SPAN_RESOURCE_FIELD.get(span);

            Resource updatedResource = resource.toBuilder()
                .put(ResourceAttributes.SERVICE_NAME, "%s-%s".formatted(
                    resource.getAttribute(ResourceAttributes.SERVICE_NAME),
                    resourceServiceNameSuffix
                ))
                .build();

            SDK_SPAN_RESOURCE_FIELD.set(span, updatedResource);

        } catch (IllegalAccessException e) {
            throw new IllegalStateException(e);
        }
    }

    // the rest is not important

}

Additional context

You have to zoom in a little, but then you can see that the e.g. redis, pdo, mongo, etc. are shown as separate services - these spans are obviously part of the app that is calling them, but the native DD agent renames the single span's service name.

It makes a lot of sense if you think about it for a bit - my database is obviously not instrumented with OTEL (maybe in the future?), so I don't have any spans from it - but I have the spans in my app that are calling the database and that is as close as I'll get in the near future to having the database calls rendered as a separate service.

image

The example image is not mine it's from some article - but it illustrates it nicely.

@fprochazka fprochazka added the Feature Request Suggest an idea for this project label Apr 23, 2023
@jkwatson
Copy link
Contributor

In general, the Resource is not something that is supposed to be mutable like this, especially not in the middle of processing a span.

If we were to add a feature like this, we'd need to have a specification update to account for it. I recommend suggesting it in the specification repository as an enhancement, as we wouldn't want to make a change this significant just for the Java implementation.

@jack-berg
Copy link
Member

If we were to add a feature like this, we'd need to have a specification update to account for it

Yup.

Although the span processor suggests general processing capabilities, the types of modifications that are possible via ReadWriteSpan are very limited. There are a few ways around this though:

  • You can define your own SpanProcessor which converts ReadWriteSpan to SpanData, then creates a wrapper version of SpanData which overwrites the resource but leaves the other fields. This processor would have to be responsible for batching and sending to an exporter.
  • You can define your own SpanExporter which for each span in a batch creates a wrapper version of SpanData overwriting the resource but leaving the other fields. Then call some other exporter with your new batch.

Neither option is super appealing, but may be preferable to hacking it with reflection.

@fprochazka
Copy link
Author

I've created the spec change request, what is the convention here? do you close this until the spec change is decided?

Also, thanks for the alternative ideas! I'll think about them :)

@jack-berg jack-berg added the blocked:spec blocked on open or unresolved spec label Apr 25, 2023
@jack-berg
Copy link
Member

We'll leave it open with the blocked:spec label.

@breedx-splk
Copy link
Contributor

Recommend linking to that spec issue here, so that it can more easily be followed up on.

@trask
Copy link
Member

trask commented Jul 20, 2023

Recommend linking to that spec issue here, so that it can more easily be followed up on.

open-telemetry/opentelemetry-specification#3433

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

5 participants