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

Spring: Cannot configure Segment name in BaseAbstractXRayInterceptor #281

Closed
stnor opened this issue Apr 30, 2021 · 15 comments
Closed

Spring: Cannot configure Segment name in BaseAbstractXRayInterceptor #281

stnor opened this issue Apr 30, 2021 · 15 comments
Assignees

Comments

@stnor
Copy link
Contributor

stnor commented Apr 30, 2021

Hi,
I'm manually instrumenting my fairly large Spring application.

Why are you not adding the Class name to the sub-segment name?
It's quite frustrating to not have the class name in the UI when looking at the traces.

I would suggest changing:

Subsegment subsegment = AWSXRay.beginSubsegment(pjp.getSignature().getName());

to

    Subsegment subsegment = AWSXRay.beginSubsegment(subsegmentNameFromSignature(pjp.getSignature()));

so that the user can override the subsegment name.

@stnor stnor changed the title Why only method in Segment name in BaseAbstractXRayInterceptor Cannot configure Segment name in BaseAbstractXRayInterceptor May 1, 2021
@stnor stnor changed the title Cannot configure Segment name in BaseAbstractXRayInterceptor Spring: Cannot configure Segment name in BaseAbstractXRayInterceptor May 1, 2021
@stnor
Copy link
Contributor Author

stnor commented May 1, 2021

I ended up rolling my own abstract class and removed the aws-xray-recorder-sdk-spring dep. It's more of an example than a library at this point, no offense.

@anuraaga
Copy link
Contributor

anuraaga commented May 2, 2021

Hi @stnor - thanks for filing these issues. It sounds like you are adding tracing to a new app. Did you by any chance take a look at our support for OpenTelemetry?

https://aws-otel.github.io/docs/getting-started/java-sdk

It has far more features and can provide a better experience than XRay sdk for new apps. There is a Java agent that can automatically instrument and/or ability to manually instrument by creating spans.

Our effort is mostly focused on OpenTelemetry going forward so we probably wouldn't be able to make changes for improving the experience here, though we would fix any bugs though

@stnor
Copy link
Contributor Author

stnor commented May 2, 2021 via email

@anuraaga
Copy link
Contributor

anuraaga commented May 2, 2021

@stnor While it hasn't quite hit GA yet it can be used in production and a few folks are doing so.

While X-Ray centralized sampling isn't currently supported unfortunately, the sampling rate can be tweaked with a flag (traceidratio).

https://github.com/open-telemetry/opentelemetry-java/tree/main/sdk-extensions/autoconfigure#sampler

@stnor
Copy link
Contributor Author

stnor commented May 2, 2021 via email

@stnor
Copy link
Contributor Author

stnor commented May 3, 2021

I've gotten the otel stuff to work decently well now... However I do not understand how to compliment the -javaagent's standard tracing with my own Spring AOP aspects.

I think the right approach would be to create nested spans? @WithSpan is not an option on 1000 methods :)
It's unclear to me how I do this. Get hold of the parent span? In the Xray Java SDK it's a oneliner:
AWSXRay.beginSubsegment(pjp.getSignature().getName());

I've looked here: https://opentelemetry.io/docs/java/manual_instrumentation/ and it doesn't really cover my use-case.

Some issue referred to OpenTelemetrySdk.getTracerProvider() but that class doesn't seem to exist anymore?
Edit: Intellj was acting up. But still don't understand how to get a ref to the current Tracer/Span or whatever I need to do to proceed ;)

I've added these deps:

        <dependency>
            <groupId>io.opentelemetry</groupId>
            <artifactId>opentelemetry-api</artifactId>
        </dependency>
        <dependency>
            <groupId>io.opentelemetry</groupId>
            <artifactId>opentelemetry-sdk</artifactId>
        </dependency>

Any pointers or help would be highly appreciated.

@stnor
Copy link
Contributor Author

stnor commented May 3, 2021

Also found open-telemetry/opentelemetry-java#3035

@stnor
Copy link
Contributor Author

stnor commented May 3, 2021

Sorry for the spam.
This code is being executed, but does nothing (that shows in Xray).

(1) Is it "correct"?
(2) If so, why doesn't the spans show up in Xray? The "standard" javaagent tracing is there still.

public abstract class AbstractSpringOtelInterceptor {

    private static final Log logger = LogFactory.getLog(AbstractSpringOtelInterceptor.class);
    private Tracer tracer = OpenTelemetrySdk.builder().build().getTracer("se.nomp");

    protected Object processXRayTrace(ProceedingJoinPoint pjp) throws Throwable {
        Span span = tracer.spanBuilder(generateSubsegmentName(pjp)).startSpan();

        try(Scope scope = span.makeCurrent()) {
            return conditionalProceed(pjp);
        } finally {
            span.end();
        }
    }

    protected String generateSubsegmentName(ProceedingJoinPoint pjp) {
        return "Hello " + pjp.getSignature().getDeclaringType().getSimpleName() + "." + pjp.getSignature().getName();
    }

    public static Object conditionalProceed(ProceedingJoinPoint pjp) throws Throwable {
        return pjp.getArgs().length == 0 ? pjp.proceed() : pjp.proceed(pjp.getArgs());
    }
}
-javaagent:/Users/stnor/devel/src/nomp/web/docker/opt/aws-opentelemetry-agent-1.1.0.jar
-Dotel.resource.attributes=service.name=Nomp
-Dotel.instrumentation.jdbc-datasource.enabled=true
-Dotel.traces.sampler.traceidratio=true
-Dotel.traces.sampler.arg=1

Also tried

    protected Object processXRayTrace(ProceedingJoinPoint pjp) throws Throwable {
        Span parent = Span.current();
        Span span = tracer.spanBuilder(generateSubsegmentName(pjp)).setParent(Context.current().with(parent)).startSpan();
        try(Scope scope = span.makeCurrent()) {
            return conditionalProceed(pjp);
        } finally {
            span.end();
        }
    }

@anuraaga
Copy link
Contributor

anuraaga commented May 4, 2021

Hey @stnor - just to confirm you are using the aws distro if the Java agent and a collector configured for x-ray? Are you seeing the agent's spans, but just not seeing yours?

To get a tracer when using the agent, you need to use GlobalOpenTelemetry.get() to get the instance of OpenTelemetry managed by the agent.

In fact, you should make sure not to have a dependency on opentelemetry-sdk in your app as the agent brings it in. The dependency itself doesn't hurt but can lead to the mistake of trying to use it. The only artifact needed for instrumenting code is opentelemetry-api.

Other than that, your first snippet (which correctly implicitly parents to the current span) looks good.

@stnor
Copy link
Contributor Author

stnor commented May 4, 2021

Thanks @anuraaga - that solved it!
I also had issues getting the aws-otel-collector to work since it did not pick up my credentials. had to do:

git clone https://github.com/aws-observability/aws-otel-collector.git ; \
    cd aws-otel-collector; \
    docker run --rm -p 4317:4317 -p 55680:55680 -p 8889:8888 \
      -e AWS_REGION=xxxx-e AWS_ACCESS_KEY_ID=xxxx -e AWS_SECRET_ACCESS_KEY=xxxx \
      -v "${PWD}/examples/docker/config-test.yaml":/otel-local-config.yaml \
      --name awscollector public.ecr.aws/aws-observability/aws-otel-collector:latest \
     --config otel-local-config.yaml;

after editing the Dockerfile.

@stnor
Copy link
Contributor Author

stnor commented May 4, 2021

Here's the result :) Thanks again.

Screenshot 2021-05-04 at 07 52 44

@anuraaga
Copy link
Contributor

anuraaga commented May 4, 2021

Nice trace!

If you found anything, always happy for more ideas on improving instrumentation at https://github.com/open-telemetry/opentelemetry-java-instrumentation/. Instrumentation for hikaricp seems like it could be nice for us to offer natively.

@anuraaga
Copy link
Contributor

anuraaga commented May 4, 2021

Ah and I've always had the idea of injecting OpenTelemetry into the spring context which I don't think we do right now. So you could use @Inject instead of GlobalOpenTelemetry.get. In the meantime you should probably do so in your app to make sure the actual tracers can (relatively) easily work if you decide to not use the agent in the future for any reason, you'd just replace the bean with an instance of OpenTelemetrySdk you build yourself.

@stnor
Copy link
Contributor Author

stnor commented May 4, 2021

I'm attaching my code here if anyone else would like to instrument att their services and repos using OpenTelemetry.

One needs to edit the within(se.nomp..*) to your package in the @Pointcut:s and also change the INSTRUMENTATION_APP_PACKAGE in AbstractSpringOpenTelemetryInterceptor
spring-service-repo-aspects.zip

@willarmiros
Copy link
Contributor

Thanks for attaching that reference @stnor! I'll go ahead and resolve this since it seems like you've resolved your issue using OpenTelemetry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants