Skip to content

Commit 0f65426

Browse files
diegomarquezpdiegomarquezp
committed
fix: use dynamic tracer name instead of hardcoded gax-java
Refactored SpanTracerFactory to dynamically initialize tracers with metadata containing the artifact name and version when withContext is invoked. Handled the null-tracer fallback strategy by returning a BaseApiTracer when tracer is null. Updated unit and integration tests.
1 parent 9d8fc97 commit 0f65426

File tree

3 files changed

+84
-51
lines changed

3 files changed

+84
-51
lines changed

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracerFactory.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import com.google.api.core.BetaApi;
3434
import com.google.api.core.InternalApi;
35+
import com.google.api.gax.rpc.LibraryMetadata;
3536
import com.google.common.annotations.VisibleForTesting;
3637
import io.opentelemetry.api.OpenTelemetry;
3738
import io.opentelemetry.api.trace.Tracer;
@@ -48,12 +49,12 @@
4849
@InternalApi
4950
public class SpanTracerFactory implements ApiTracerFactory {
5051
private final Tracer tracer;
51-
52+
private final OpenTelemetry openTelemetry;
5253
private final ApiTracerContext apiTracerContext;
5354

5455
/** Creates a SpanTracerFactory */
5556
public SpanTracerFactory(OpenTelemetry openTelemetry) {
56-
this(openTelemetry.getTracer("gax-java"), ApiTracerContext.empty());
57+
this(openTelemetry, null, ApiTracerContext.empty());
5758
}
5859

5960
/**
@@ -62,13 +63,18 @@ public SpanTracerFactory(OpenTelemetry openTelemetry) {
6263
* internally.
6364
*/
6465
@VisibleForTesting
65-
SpanTracerFactory(Tracer tracer, ApiTracerContext apiTracerContext) {
66+
SpanTracerFactory(OpenTelemetry openTelemetry, Tracer tracer, ApiTracerContext apiTracerContext) {
67+
this.openTelemetry = openTelemetry;
6668
this.tracer = tracer;
6769
this.apiTracerContext = apiTracerContext;
6870
}
6971

7072
@Override
7173
public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) {
74+
if (tracer == null) {
75+
// Return a no-op tracer if withContext hasn't been called to initialize the tracer properly
76+
return new BaseApiTracer();
77+
}
7278
// TODO(diegomarquezp): this is a placeholder for span names and will be adjusted as the
7379
// feature is developed.
7480
String attemptSpanName = spanName.getClientName() + "/" + spanName.getMethodName() + "/attempt";
@@ -78,6 +84,10 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
7884

7985
@Override
8086
public ApiTracer newTracer(ApiTracer parent, ApiTracerContext apiTracerContext) {
87+
if (tracer == null) {
88+
// Return a no-op tracer if withContext hasn't been called to initialize the tracer properly
89+
return new BaseApiTracer();
90+
}
8191
ApiTracerContext mergedContext = this.apiTracerContext.merge(apiTracerContext);
8292
return new SpanTracer(tracer, mergedContext);
8393
}
@@ -87,8 +97,18 @@ public ApiTracerContext getApiTracerContext() {
8797
return apiTracerContext;
8898
}
8999

100+
/**
101+
* Returns a new SpanTracerFactory with the provided context. The Tracer is re-initialized using
102+
* the artifact name and version from the library metadata.
103+
*/
90104
@Override
91105
public ApiTracerFactory withContext(ApiTracerContext context) {
92-
return new SpanTracerFactory(tracer, apiTracerContext.merge(context));
106+
ApiTracerContext mergedContext = this.apiTracerContext.merge(context);
107+
LibraryMetadata metadata = mergedContext.libraryMetadata();
108+
// Using io.opentelemetry.api.trace.Tracer.getTracer(String instrumentationScopeName, String
109+
// instrumentationScopeVersion)
110+
// This allows us to specify both the artifact name and version for better observability.
111+
Tracer newTracer = openTelemetry.getTracer(metadata.artifactName(), metadata.version());
112+
return new SpanTracerFactory(openTelemetry, newTracer, mergedContext);
93113
}
94114
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static com.google.common.truth.Truth.assertThat;
3434
import static org.mockito.ArgumentMatchers.any;
3535
import static org.mockito.ArgumentMatchers.anyString;
36+
import static org.mockito.ArgumentMatchers.nullable;
3637
import static org.mockito.Mockito.atLeastOnce;
3738
import static org.mockito.Mockito.mock;
3839
import static org.mockito.Mockito.verify;
@@ -41,6 +42,7 @@
4142
import com.google.api.gax.rpc.LibraryMetadata;
4243
import com.google.api.gax.tracing.ApiTracerContext.Transport;
4344
import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
45+
import io.opentelemetry.api.OpenTelemetry;
4446
import io.opentelemetry.api.common.AttributeKey;
4547
import io.opentelemetry.api.common.Attributes;
4648
import io.opentelemetry.api.trace.Span;
@@ -54,15 +56,19 @@
5456
import org.mockito.ArgumentCaptor;
5557

5658
class SpanTracerFactoryTest {
59+
private OpenTelemetry openTelemetry;
5760
private Tracer tracer;
5861
private SpanBuilder spanBuilder;
5962
private Span span;
6063

6164
@BeforeEach
6265
void setUp() {
66+
openTelemetry = mock(OpenTelemetry.class);
6367
tracer = mock(Tracer.class);
6468
spanBuilder = mock(SpanBuilder.class);
6569
span = mock(Span.class);
70+
when(openTelemetry.getTracer(nullable(String.class), nullable(String.class)))
71+
.thenReturn(tracer);
6672
when(tracer.spanBuilder(anyString())).thenReturn(spanBuilder);
6773
when(spanBuilder.setSpanKind(any())).thenReturn(spanBuilder);
6874
when(spanBuilder.setAllAttributes(any(Attributes.class))).thenReturn(spanBuilder);
@@ -72,7 +78,8 @@ void setUp() {
7278
@ParameterizedTest
7379
@ValueSource(booleans = {false, true})
7480
void testNewTracer_createsSpanTracer(boolean useContext) {
75-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
81+
SpanTracerFactory factory =
82+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
7683
ApiTracer tracerInstance;
7784
if (useContext) {
7885
ApiTracerContext context =
@@ -92,7 +99,8 @@ void testNewTracer_createsSpanTracer(boolean useContext) {
9299
@ParameterizedTest
93100
@ValueSource(booleans = {false, true})
94101
void testNewTracer_addsAttributes(boolean useContext) {
95-
ApiTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
102+
ApiTracerFactory factory =
103+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
96104
factory =
97105
factory.withContext(
98106
ApiTracerContext.newBuilder()
@@ -132,7 +140,8 @@ void testWithContext_addsInferredAttributes(boolean useContext) {
132140
.setServerAddress("example.com")
133141
.build();
134142

135-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
143+
SpanTracerFactory factory =
144+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
136145
ApiTracerFactory factoryWithContext = factory.withContext(context);
137146

138147
ApiTracer tracerInstance;
@@ -166,7 +175,8 @@ void testWithContext_addsInferredAttributes(boolean useContext) {
166175
void testWithContext_noEndpointContext_doesNotAddServerAddressAttribute(boolean useContext) {
167176
ApiTracerContext context = ApiTracerContext.empty();
168177

169-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
178+
SpanTracerFactory factory =
179+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
170180
ApiTracerFactory factoryWithContext = factory.withContext(context);
171181

172182
ApiTracer tracerInstance;
@@ -203,7 +213,8 @@ void testNewTracer_withContext_grpc_usesFullMethodName() {
203213
.setLibraryMetadata(LibraryMetadata.empty())
204214
.build();
205215

206-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
216+
SpanTracerFactory factory =
217+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
207218
ApiTracer tracerInstance = factory.newTracer(null, context);
208219

209220
tracerInstance.attemptStarted(null, 1);
@@ -229,7 +240,8 @@ void testNewTracer_withContext_http_usesHttpMethodAndPathTemplate(
229240
.setLibraryMetadata(LibraryMetadata.empty())
230241
.build();
231242

232-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
243+
SpanTracerFactory factory =
244+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
233245
ApiTracer tracerInstance = factory.newTracer(null, context);
234246

235247
tracerInstance.attemptStarted(null, 1);
@@ -246,7 +258,8 @@ void testNewTracer_withContext_http_noHttpMethodOrPathTemplate_usesFullMethodNam
246258
.setLibraryMetadata(LibraryMetadata.empty())
247259
.build();
248260

249-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
261+
SpanTracerFactory factory =
262+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
250263
ApiTracer tracerInstance = factory.newTracer(null, context);
251264

252265
tracerInstance.attemptStarted(null, 1);
@@ -256,7 +269,8 @@ void testNewTracer_withContext_http_noHttpMethodOrPathTemplate_usesFullMethodNam
256269

257270
@Test
258271
void testNewTracer_withSpanName_usesPlaceholder() {
259-
SpanTracerFactory factory = new SpanTracerFactory(tracer, ApiTracerContext.empty());
272+
SpanTracerFactory factory =
273+
new SpanTracerFactory(openTelemetry, tracer, ApiTracerContext.empty());
260274
ApiTracer tracerInstance =
261275
factory.newTracer(null, SpanName.of("Service", "Method"), OperationType.Unary);
262276

@@ -272,7 +286,7 @@ void testNewTracer_mergesFactoryContext() {
272286
.setServerAddress("factory-address")
273287
.setLibraryMetadata(LibraryMetadata.empty())
274288
.build();
275-
SpanTracerFactory factory = new SpanTracerFactory(tracer, apiTracerContext);
289+
SpanTracerFactory factory = new SpanTracerFactory(openTelemetry, tracer, apiTracerContext);
276290

277291
ApiTracerContext callContext =
278292
ApiTracerContext.newBuilder()
@@ -293,4 +307,19 @@ void testNewTracer_mergesFactoryContext() {
293307
assertThat(attributes.asMap())
294308
.containsEntry(AttributeKey.stringKey("rpc.method"), "Service/Method");
295309
}
310+
311+
@Test
312+
void testNoOpWhenTracerNull() {
313+
SpanTracerFactory factory =
314+
new SpanTracerFactory(openTelemetry, null, ApiTracerContext.empty());
315+
316+
ApiTracer tracerInstance =
317+
factory.newTracer(null, SpanName.of("Service", "Method"), OperationType.Unary);
318+
319+
assertThat(tracerInstance).isInstanceOf(BaseApiTracer.class);
320+
321+
ApiTracer tracerInstance2 = factory.newTracer(null, ApiTracerContext.empty());
322+
323+
assertThat(tracerInstance2).isInstanceOf(BaseApiTracer.class);
324+
}
296325
}

‎sdk-platform-java/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java‎

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@
4545
import com.google.showcase.v1beta1.EchoClient;
4646
import com.google.showcase.v1beta1.EchoRequest;
4747
import com.google.showcase.v1beta1.EchoSettings;
48-
import com.google.showcase.v1beta1.GetUserRequest;
49-
import com.google.showcase.v1beta1.IdentityClient;
5048
import com.google.showcase.v1beta1.it.util.TestClientInitializer;
5149
import com.google.showcase.v1beta1.stub.EchoStub;
5250
import com.google.showcase.v1beta1.stub.EchoStubSettings;
@@ -94,24 +92,20 @@ void tearDown() {
9492
}
9593

9694
@Test
97-
void testTracing_successfulIdentityGetUser_grpc() throws Exception {
95+
void testTracing_successfulEcho_grpc() throws Exception {
9896
SpanTracerFactory tracingFactory = new SpanTracerFactory(openTelemetrySdk);
9997

100-
try (IdentityClient client =
101-
TestClientInitializer.createGrpcIdentityClientOpentelemetry(tracingFactory)) {
98+
try (EchoClient client =
99+
TestClientInitializer.createGrpcEchoClientOpentelemetry(tracingFactory)) {
102100

103-
try {
104-
client.getUser(GetUserRequest.newBuilder().setName("users/test-user").build());
105-
} catch (Exception e) {
106-
// Ignored, the showcase server may not have this user, but trace is still generated.
107-
}
101+
client.echo(EchoRequest.newBuilder().setContent("tracing-test").build());
108102

109103
List<SpanData> spans = spanExporter.getFinishedSpanItems();
110104
assertThat(spans).isNotEmpty();
111105

112106
SpanData attemptSpan =
113107
spans.stream()
114-
.filter(span -> span.getName().equals("google.showcase.v1beta1.Identity/GetUser"))
108+
.filter(span -> span.getName().equals("google.showcase.v1beta1.Echo/Echo"))
115109
.findFirst()
116110
.orElseThrow(() -> new AssertionError("Incorrect span name"));
117111
assertThat(attemptSpan.getKind()).isEqualTo(SpanKind.CLIENT);
@@ -149,46 +143,38 @@ void testTracing_successfulIdentityGetUser_grpc() throws Exception {
149143
attemptSpan
150144
.getAttributes()
151145
.get(AttributeKey.stringKey(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE)))
152-
.isEqualTo("google.showcase.v1beta1.Identity/GetUser");
146+
.isEqualTo("google.showcase.v1beta1.Echo/Echo");
147+
assertThat(attemptSpan.getInstrumentationScopeInfo().getName()).isEqualTo(SHOWCASE_ARTIFACT);
153148
// {x-version-update-start:gapic-showcase:current}
154149
assertThat(
155150
attemptSpan
156151
.getAttributes()
157152
.get(AttributeKey.stringKey(ObservabilityAttributes.VERSION_ATTRIBUTE)))
158153
.isEqualTo("0.0.0-SNAPSHOT");
154+
assertThat(attemptSpan.getInstrumentationScopeInfo().getVersion())
155+
.isEqualTo("0.0.0-SNAPSHOT");
159156
// {x-version-update-end}
160-
assertThat(
161-
attemptSpan
162-
.getAttributes()
163-
.get(
164-
AttributeKey.stringKey(
165-
ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE)))
166-
.isEqualTo("users/test-user");
167157
}
168158
}
169159

170160
@Test
171-
void testTracing_successfulIdentityGetUser_httpjson() throws Exception {
161+
void testTracing_successfulEcho_httpjson() throws Exception {
172162
SpanTracerFactory tracingFactory = new SpanTracerFactory(openTelemetrySdk);
173163

174-
try (IdentityClient client =
175-
TestClientInitializer.createHttpJsonIdentityClientOpentelemetry(tracingFactory)) {
164+
try (EchoClient client =
165+
TestClientInitializer.createHttpJsonEchoClientOpentelemetry(tracingFactory)) {
176166

177-
try {
178-
client.getUser(GetUserRequest.newBuilder().setName("users/test-user").build());
179-
} catch (Exception e) {
180-
// Ignored, the showcase server may not have this user, but trace is still generated.
181-
}
167+
client.echo(EchoRequest.newBuilder().setContent("tracing-test").build());
182168

183169
List<SpanData> spans = spanExporter.getFinishedSpanItems();
184170
assertThat(spans).isNotEmpty();
185171

186172
SpanData attemptSpan =
187173
spans.stream()
188-
.filter(span -> span.getName().equals("GET v1beta1/{name=users/*}"))
174+
.filter(span -> span.getName().equals("POST v1beta1/echo:echo"))
189175
.findFirst()
190176
.orElseThrow(
191-
() -> new AssertionError("Attempt span 'GET v1beta1/{name=users/*}' not found"));
177+
() -> new AssertionError("Attempt span 'POST v1beta1/echo:echo' not found"));
192178
assertThat(attemptSpan.getKind()).isEqualTo(SpanKind.CLIENT);
193179
assertThat(
194180
attemptSpan
@@ -219,19 +205,17 @@ void testTracing_successfulIdentityGetUser_httpjson() throws Exception {
219205
attemptSpan
220206
.getAttributes()
221207
.get(AttributeKey.stringKey(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE)))
222-
.isEqualTo("GET");
208+
.isEqualTo("POST");
223209
assertThat(
224210
attemptSpan
225211
.getAttributes()
226212
.get(AttributeKey.stringKey(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE)))
227-
.isEqualTo("v1beta1/{name=users/*}");
228-
assertThat(
229-
attemptSpan
230-
.getAttributes()
231-
.get(
232-
AttributeKey.stringKey(
233-
ObservabilityAttributes.DESTINATION_RESOURCE_ID_ATTRIBUTE)))
234-
.isEqualTo("users/test-user");
213+
.isEqualTo("v1beta1/echo:echo");
214+
assertThat(attemptSpan.getInstrumentationScopeInfo().getName()).isEqualTo(SHOWCASE_ARTIFACT);
215+
// {x-version-update-start:gapic-showcase:current}
216+
assertThat(attemptSpan.getInstrumentationScopeInfo().getVersion())
217+
.isEqualTo("0.0.0-SNAPSHOT");
218+
// {x-version-update-end}
235219
}
236220
}
237221

0 commit comments

Comments
 (0)