Add instrumentation for thrift by laurit · Pull Request #18405 · open-telemetry/opentelemetry-java-instrumentation

laurit · 2026-04-29T08:16:33Z

Resolves #1573
While reviewing #13784 noticed that it is significantly more complicated than https://github.com/opentracing-contrib/java-thrift Here is my attempt at building a simpler instrumentation.
It is possible that some of the complexity in #13784 is because of supporting 0.9.1 vs 0.13.0 in the opentracing. Could also be that it handles some cases better. I didn't get around to trying the tests from #13784 with my instrumentation. The tests in this PR are based on the opentracing tests.
cc @YaoYingLong

laurit · 2026-04-29T16:17:28Z

+  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
+    // limit support to 0.13.0 since we don't test with earlier versions
+    // added in 0.13.0
+    return hasClassesNamed("org.apache.thrift.protocol.ShortStack");


While the library instrumentation requires 0.13.0 the agent instrumentation could work with older versions. We are limiting the version here because we don't test on older versions.

trask

Automated CI review from #17889 incoming...

Review of the new Apache Thrift instrumentation. Main findings:

SocketAccessor has a copy/paste bug: FRAMED_TRANSPORT and FAST_FRAMED_TRANSPORT both resolve to TFastFramedTransport, so TFramedTransport-wrapped transports never reach the framed-transport branch and their socket is not unwrapped.
ContextPropagationUtil.readHeaders corrupts the protocol stream when the entry count exceeds MAX_CONTEXT_ENTRIES — it returns without skipping the unread map entries, while the size-limit branch correctly skips remaining entries.
library/README.md looks copy-pasted from gRPC: wrong title, wrong central.sonatype.com artifact link, undefined telemetry variable in the snippets, wrong return type on wrapClientProtocolFactory.
Minor style: this.getClass().getName() in transform() should be getClass().getName() (per javaagent-module-patterns.md); duplicate AutoCleanupExtension field in the library ThriftTest (the abstract base already declares one).

…when running with agent

trask · 2026-05-04T20:44:23Z

I sent a few optional suggestions in laurit#410

# Conflicts: # .github/config/latest-dep-versions.json

Copilot

Pull request overview

This PR introduces Apache Thrift 0.13+ instrumentation to the OpenTelemetry Java Instrumentation project, providing both a standalone library wrapper (ThriftTelemetry) and Javaagent auto-instrumentation, along with a dedicated testing module and documentation/metadata updates.

Changes:

Added new Thrift 0.13 modules: library, javaagent, and testing, including client/server span + metric coverage and context propagation.
Implemented Thrift client/server wrappers for library users and ByteBuddy instrumentations for javaagent users.
Added Thrift test coverage (sync, async, transports), plus repo metadata/docs updates and latest-deps tracking.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`settings.gradle.kts`	Registers the new Thrift 0.13 modules in the Gradle build.
`instrumentation/thrift-0.13/testing/src/main/thrift/custom.thrift`	Defines the Thrift IDL used to generate test client/server artifacts.
`instrumentation/thrift-0.13/testing/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/CustomHandler.java`	Implements the Thrift service used by tests.
`instrumentation/thrift-0.13/testing/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/AbstractThriftTest.java`	Shared test suite validating spans/metrics and behavior across server/client modes.
`instrumentation/thrift-0.13/testing/build.gradle.kts`	Adds the testing module dependencies and Thrift source generation step.
`instrumentation/thrift-0.13/metadata.yaml`	Declares instrumentation metadata (semantic conventions, description, link).
`instrumentation/thrift-0.13/library/src/test/java/io/opentelemetry/instrumentation/thrift/v0_13/ThriftTest.java`	Library-instrumentation test coverage using `ThriftTelemetry` wrappers.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/ThriftTelemetryBuilder.java`	Builder API to customize extractors and span naming for Thrift telemetry.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/ThriftTelemetry.java`	Public entrypoint providing wrappers for Thrift processors/protocols/clients.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/ThriftResponse.java`	Response type used for instrumenter end/status decisions.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/ThriftRequest.java`	Request carrier for RPC + network attributes and propagation headers.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ThriftServerNetworkAttributesGetter.java`	Extracts server-side network/server attributes from `ThriftRequest`.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ThriftRpcAttributesGetter.java`	Provides RPC semantic convention attributes (system/service/method/error).
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ThriftRequestGetter.java`	TextMapGetter for extracting propagation headers from `ThriftRequest`.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ThriftInstrumenterFactory.java`	Builds client/server instrumenters and wires RPC/network extractors + metrics.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ThriftClientNetworkAttributesGetter.java`	Extracts client-side server/network attributes from `ThriftRequest`.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/SocketAccessor.java`	Retrieves socket/socket address details across Thrift transport variants.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ServerProcessorDecorator.java`	Wraps server processor to apply protocol decorators and end spans correctly.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ServerOutProtocolDecorator.java`	Detects server-side exception responses when writing messages.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ServerInProtocolDecorator.java`	Reads propagation field, starts server span, and scopes context for handlers.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ServerCallContext.java`	ThreadLocal context to expose transport for nonblocking server socket discovery.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ContextPropagationUtil.java`	Writes/reads propagation payload in Thrift maps with safety limits.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ClientProtocolDecorator.java`	Injects propagation headers into outbound Thrift message fields.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ClientCallContext.java`	ThreadLocal call context to coordinate injection and client span lifecycle.
`instrumentation/thrift-0.13/library/src/main/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/AsyncMethodCallbackUtil.java`	Wraps async callbacks to end spans and restore parent context for callback code.
`instrumentation/thrift-0.13/library/README.md`	Documents how to use the standalone Thrift library instrumentation.
`instrumentation/thrift-0.13/library/build.gradle.kts`	Declares library instrumentation build/test tasks (including semconv variants).
`instrumentation/thrift-0.13/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftTest.java`	Javaagent test coverage via `AbstractThriftTest`.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftTBaseProcessorInstrumentation.java`	Instruments server processing entrypoint for blocking servers.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftSingletons.java`	Provides global client/server instrumenters and propagators for the agent.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftServiceClientInstrumentation.java`	Instruments generated Thrift sync clients and wraps protocols for propagation.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftInstrumentationModule.java`	Registers the Thrift agent instrumentation module and its type instrumentations.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftFrameBufferInstrumentation.java`	Adds transport exposure for nonblocking server processing via ThreadLocal context.
`instrumentation/thrift-0.13/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/thrift/v0_13/ThriftAsyncClientInstrumentation.java`	Instruments generated Thrift async clients, wrapping callbacks for span end.
`instrumentation/thrift-0.13/javaagent/build.gradle.kts`	Configures agent module dependencies, muzzle, and semconv-variant test tasks.
`docs/supported-libraries.md`	Adds Thrift to the supported libraries list.
`.github/config/latest-dep-versions.json`	Adds `org.apache.thrift:libthrift` latest version for dependency update tooling.
`.fossa.yml`	Adds new Thrift gradle targets for license scanning.

Copilot

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.

trask · 2026-05-10T23:04:07Z

+    transformer.applyAdviceToMethod(
+        isConstructor().and(takesArgument(0, named("org.apache.thrift.protocol.TProtocolFactory"))),
+        getClass().getName() + "$ConstructorAdvice");
+
+    transformer.applyAdviceToMethod(
+        isMethod()
+            .and(
+                target -> {
+                  ParameterList<?> parameterList = target.getParameters();
+                  if (!parameterList.isEmpty()) {
+                    String lastParameter =
+                        parameterList.get(parameterList.size() - 1).getType().asErasure().getName();
+                    return "org.apache.thrift.async.AsyncMethodCallback".equals(lastParameter);
+                  }
+                  return false;
+                }),
+        getClass().getName() + "$MethodAdvice");
+  }
+
+  @SuppressWarnings("unused")
+  public static class ConstructorAdvice {
+    @Advice.AssignReturned.ToArguments(@ToArgument(0))
+    @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+    public static TProtocolFactory onEnter(
+        @Advice.Argument(0) TProtocolFactory protocolFactory,
+        @Advice.Argument(2) TNonblockingTransport transport) {
+      return new ClientProtocolDecorator.Factory(protocolFactory, getPropagators());


Fixed in fa3f3bc — tightened the matcher with takesArguments(3) and takesArgument(2, TNonblockingTransport), and dropped the unused transport advice binding.

Update: amended in 951e709 — kept just the original takesArgument(0, TProtocolFactory) matcher and only removed the unused @Advice.Argument(2) TNonblockingTransport binding. takesArguments(3) would have over-tightened (Thrift's generator emits both 3-arg and 4-arg subclass ctors via super(...), so both should be instrumented to wrap the protocol factory).

Copilot

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.

laurit · 2026-05-11T14:04:08Z

@copilot apply changes based on the comments in this thread

Copilot

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.

laurit · 2026-05-12T14:31:26Z

+  private UnaryOperator<SpanNameExtractor<ThriftRequest>> clientSpanNameExtractorCustomizer =
+      UnaryOperator.identity();
+  private UnaryOperator<SpanNameExtractor<ThriftRequest>> serverSpanNameExtractorCustomizer =
+      UnaryOperator.identity();
+  private final List<AttributesExtractor<ThriftRequest, ThriftResponse>> additionalExtractors =
+      new ArrayList<>();
+  private final List<AttributesExtractor<ThriftRequest, ThriftResponse>>
+      additionalClientExtractors = new ArrayList<>();
+  private final List<AttributesExtractor<ThriftRequest, ThriftResponse>>
+      additionalServerExtractors = new ArrayList<>();


We removed the complex generics from http instrumentations, should we also do it elsewhere for 3.0?

Add instrumentation for thrift

57a948b

otelbot-java-instrumentation Bot added the test native This label can be applied to PRs to trigger them to run native tests label Apr 29, 2026

laurit added 4 commits April 29, 2026 11:29

spotless

ad040ca

restore check

7a548ea

fixes

7341770

remove commented out code

8b4aa72

laurit commented Apr 29, 2026

View reviewed changes

laurit marked this pull request as ready for review April 29, 2026 19:52

laurit requested a review from a team as a code owner April 29, 2026 19:52

trask mentioned this pull request Apr 30, 2026

Pull Request Dashboard #18435

Open

trask reviewed Apr 30, 2026

View reviewed changes

trask mentioned this pull request Apr 30, 2026

feat: Add support for Thrift 0.9.1 and later versions #13784

Closed

laurit added 3 commits May 4, 2026 09:26

address review comments, capture network attributes for async server …

dc8872a

…when running with agent

Merge branch 'main' into thrift

7c26fff

fix link

8eabed7

trask approved these changes May 4, 2026

View reviewed changes

Comment thread ...ain/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ClientProtocolDecorator.java

Comment thread ...ain/java/io/opentelemetry/instrumentation/thrift/v0_13/internal/ClientProtocolDecorator.java Outdated

trask mentioned this pull request May 4, 2026

Thrift instrumentation simplifications laurit/opentelemetry-java-instrumentation#410

Open

github-actions Bot mentioned this pull request May 5, 2026

Pull Request Dashboard #18604

Closed

laurit and others added 4 commits May 6, 2026 15:43

redo client instrumentation

8e2adf6

error test

f2f2248

Merge remote-tracking branch 'upstream/main' into thrift

b758f2c

# Conflicts: # .github/config/latest-dep-versions.json

Merge branch 'main' into thrift

555b456

laurit modified the milestone: v3.0.0 May 7, 2026

trask requested a review from Copilot May 10, 2026 00:14

Copilot started reviewing on behalf of trask May 10, 2026 00:15 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

trask added 2 commits May 9, 2026 17:52

Skip instrumentation for Object methods on client proxies

54f5a13

End server span when message parsing fails after start

d4daa19

trask added 2 commits May 9, 2026 17:52

Clean up async client transport and manager in tests

5605905

Link thrift library instrumentation in supported-libraries

2e059f5

laurit requested a review from Copilot May 10, 2026 15:04

Copilot started reviewing on behalf of laurit May 10, 2026 15:04 View session

Copilot AI reviewed May 10, 2026

View reviewed changes

trask added 2 commits May 10, 2026 16:03

Fix thrift library path in supported-libraries.md

478a4d1

Pass openTelemetry parameter in README client wrapper examples

f4e6b67

trask force-pushed the thrift branch from fa3f3bc to 92b85c1 Compare May 11, 2026 01:18

Drop unused transport binding from TAsyncClient constructor advice

951e709

trask force-pushed the thrift branch from 92b85c1 to 951e709 Compare May 11, 2026 01:18

trask requested a review from Copilot May 11, 2026 01:19

Copilot started reviewing on behalf of trask May 11, 2026 01:20 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

address review comment

daeaa20

laurit added 2 commits May 12, 2026 11:04

add context propagation field to top level struct

ae2c6cd

Merge branch 'main' into thrift

63e85d3

laurit added this to the v2.28.0 milestone May 12, 2026

laurit requested a review from Copilot May 12, 2026 08:07

Copilot started reviewing on behalf of laurit May 12, 2026 08:08 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

laurit added 2 commits May 12, 2026 11:19

add context propagation field to top level struct

feb4a6a

address review comments

1907f82

trask merged commit e3d5372 into open-telemetry:main May 16, 2026
96 checks passed

Conversation

laurit commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laurit Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

trask left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trask commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

trask May 10, 2026

Choose a reason for hiding this comment

Uh oh!

trask May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laurit commented May 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

laurit May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

laurit commented Apr 29, 2026 •

edited

Loading