Skip to content

Commit 4532648

Browse files
dmarkwatdmarkwat
authored
Addresses opencensus-shim trace issues under otel javaagent (#4900)
* Improves opencensus-shim javaagent interop * Fix name * relocate to opencensusshim package * unit tests for ProxyingSpan * cleans up unnecessary tests and adds comments * restrict non-public interfaces * PR feedback
1 parent 0298989 commit 4532648

File tree

3 files changed

+385
-62
lines changed

3 files changed

+385
-62
lines changed

‎opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/DelegatingSpan.java

+164
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.opencensusshim;
7+
8+
import com.google.errorprone.annotations.MustBeClosed;
9+
import io.opentelemetry.api.common.AttributeKey;
10+
import io.opentelemetry.api.common.Attributes;
11+
import io.opentelemetry.api.trace.Span;
12+
import io.opentelemetry.api.trace.SpanContext;
13+
import io.opentelemetry.api.trace.StatusCode;
14+
import io.opentelemetry.context.Context;
15+
import io.opentelemetry.context.Scope;
16+
import java.time.Instant;
17+
import java.util.concurrent.TimeUnit;
18+
19+
/**
20+
* Delegates <i>all</i> {@link Span} methods to some underlying Span via {@link
21+
* DelegatingSpan#getDelegate()}.
22+
*
23+
* <p>If not all calls are proxied, some, such as and in particular {@link
24+
* Span#storeInContext(Context)} and {@link Span#makeCurrent()}, will use {@code this} instead of
25+
* the proxied {@link Span} which betrays the expectation of instance fidelity imposed by the
26+
* underlying otel mechanisms which minted the original {@link Span}, such as the otel javaagent.
27+
*
28+
* <p>This proxy class simplification allows the shim to perform its duties as minimally invasively
29+
* as possible and itself never expose its own classes and objects to callers or recipients of calls
30+
* from the shim.
31+
*
32+
* <p>This addresses the inconsistency where not all methods are appropriately delegated by exposing
33+
* a single method, {@link DelegatingSpan#getDelegate()}, to simplify and better ensure delegation
34+
* and meeting expectations.
35+
*/
36+
interface DelegatingSpan extends Span {
37+
Span getDelegate();
38+
39+
@Override
40+
default Span updateName(String name) {
41+
return getDelegate().updateName(name);
42+
}
43+
44+
@Override
45+
default SpanContext getSpanContext() {
46+
return getDelegate().getSpanContext();
47+
}
48+
49+
@Override
50+
default boolean isRecording() {
51+
return getDelegate().isRecording();
52+
}
53+
54+
@Override
55+
default <T> Span setAttribute(AttributeKey<T> key, T value) {
56+
return getDelegate().setAttribute(key, value);
57+
}
58+
59+
@Override
60+
default Span setAttribute(String key, String value) {
61+
return getDelegate().setAttribute(key, value);
62+
}
63+
64+
@Override
65+
default Span setAttribute(String key, long value) {
66+
return getDelegate().setAttribute(key, value);
67+
}
68+
69+
@Override
70+
default Span setAttribute(String key, double value) {
71+
return getDelegate().setAttribute(key, value);
72+
}
73+
74+
@Override
75+
default Span setAttribute(String key, boolean value) {
76+
return getDelegate().setAttribute(key, value);
77+
}
78+
79+
@Override
80+
default Span setAttribute(AttributeKey<Long> key, int value) {
81+
return getDelegate().setAttribute(key, value);
82+
}
83+
84+
@Override
85+
default Span setAllAttributes(Attributes attributes) {
86+
return getDelegate().setAllAttributes(attributes);
87+
}
88+
89+
@Override
90+
default Span addEvent(String name, Attributes attributes) {
91+
return getDelegate().addEvent(name, attributes);
92+
}
93+
94+
@Override
95+
default Span addEvent(String name, Attributes attributes, long timestamp, TimeUnit unit) {
96+
return getDelegate().addEvent(name, attributes, timestamp, unit);
97+
}
98+
99+
@Override
100+
default Span addEvent(String name) {
101+
return getDelegate().addEvent(name);
102+
}
103+
104+
@Override
105+
default Span addEvent(String name, long timestamp, TimeUnit unit) {
106+
return getDelegate().addEvent(name, timestamp, unit);
107+
}
108+
109+
@Override
110+
default Span addEvent(String name, Instant timestamp) {
111+
return getDelegate().addEvent(name, timestamp);
112+
}
113+
114+
@Override
115+
default Span addEvent(String name, Attributes attributes, Instant timestamp) {
116+
return getDelegate().addEvent(name, attributes, timestamp);
117+
}
118+
119+
@Override
120+
default Span setStatus(StatusCode statusCode, String description) {
121+
return getDelegate().setStatus(statusCode, description);
122+
}
123+
124+
@Override
125+
default Span setStatus(StatusCode statusCode) {
126+
return getDelegate().setStatus(statusCode);
127+
}
128+
129+
@Override
130+
default Span recordException(Throwable exception, Attributes additionalAttributes) {
131+
return getDelegate().recordException(exception, additionalAttributes);
132+
}
133+
134+
@Override
135+
default Span recordException(Throwable exception) {
136+
return getDelegate().recordException(exception);
137+
}
138+
139+
@Override
140+
default void end(Instant timestamp) {
141+
getDelegate().end(timestamp);
142+
}
143+
144+
@Override
145+
default void end() {
146+
getDelegate().end();
147+
}
148+
149+
@Override
150+
default void end(long timestamp, TimeUnit unit) {
151+
getDelegate().end(timestamp, unit);
152+
}
153+
154+
@Override
155+
default Context storeInContext(Context context) {
156+
return getDelegate().storeInContext(context);
157+
}
158+
159+
@MustBeClosed
160+
@Override
161+
default Scope makeCurrent() {
162+
return getDelegate().makeCurrent();
163+
}
164+
}

‎opencensus-shim/src/main/java/io/opentelemetry/opencensusshim/OpenTelemetrySpanImpl.java

+22-62
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,10 @@
4646
import io.opentelemetry.api.trace.StatusCode;
4747
import java.util.EnumSet;
4848
import java.util.Map;
49-
import java.util.concurrent.TimeUnit;
5049
import java.util.logging.Logger;
51-
import javax.annotation.Nonnull;
5250

53-
class OpenTelemetrySpanImpl extends Span implements io.opentelemetry.api.trace.Span {
51+
class OpenTelemetrySpanImpl extends Span
52+
implements io.opentelemetry.api.trace.Span, DelegatingSpan {
5453
private static final Logger LOGGER = Logger.getLogger(OpenTelemetrySpanImpl.class.getName());
5554
private static final EnumSet<Span.Options> RECORD_EVENTS_SPAN_OPTIONS =
5655
EnumSet.of(Span.Options.RECORD_EVENTS);
@@ -62,15 +61,24 @@ class OpenTelemetrySpanImpl extends Span implements io.opentelemetry.api.trace.S
6261
this.otelSpan = otelSpan;
6362
}
6463

64+
// otel
65+
66+
@Override
67+
public io.opentelemetry.api.trace.Span getDelegate() {
68+
return otelSpan;
69+
}
70+
71+
// opencensus
72+
6573
@Override
6674
public void putAttribute(String key, AttributeValue value) {
6775
Preconditions.checkNotNull(key, "key");
6876
Preconditions.checkNotNull(value, "value");
6977
value.match(
70-
arg -> otelSpan.setAttribute(key, arg),
71-
arg -> otelSpan.setAttribute(key, arg),
72-
arg -> otelSpan.setAttribute(key, arg),
73-
arg -> otelSpan.setAttribute(key, arg),
78+
arg -> DelegatingSpan.super.setAttribute(key, arg),
79+
arg -> DelegatingSpan.super.setAttribute(key, arg),
80+
arg -> DelegatingSpan.super.setAttribute(key, arg),
81+
arg -> DelegatingSpan.super.setAttribute(key, arg),
7482
arg -> null);
7583
}
7684

@@ -84,14 +92,14 @@ public void putAttributes(Map<String, AttributeValue> attributes) {
8492
public void addAnnotation(String description, Map<String, AttributeValue> attributes) {
8593
AttributesBuilder attributesBuilder = Attributes.builder();
8694
mapAttributes(attributes, attributesBuilder);
87-
otelSpan.addEvent(description, attributesBuilder.build());
95+
DelegatingSpan.super.addEvent(description, attributesBuilder.build());
8896
}
8997

9098
@Override
9199
public void addAnnotation(Annotation annotation) {
92100
AttributesBuilder attributesBuilder = Attributes.builder();
93101
mapAttributes(annotation.getAttributes(), attributesBuilder);
94-
otelSpan.addEvent(annotation.getDescription(), attributesBuilder.build());
102+
DelegatingSpan.super.addEvent(annotation.getDescription(), attributesBuilder.build());
95103
}
96104

97105
@Override
@@ -101,7 +109,7 @@ public void addLink(Link link) {
101109

102110
@Override
103111
public void addMessageEvent(MessageEvent messageEvent) {
104-
otelSpan.addEvent(
112+
DelegatingSpan.super.addEvent(
105113
String.valueOf(messageEvent.getMessageId()),
106114
Attributes.of(
107115
AttributeKey.stringKey(MESSAGE_EVENT_ATTRIBUTE_KEY_TYPE),
@@ -115,70 +123,22 @@ public void addMessageEvent(MessageEvent messageEvent) {
115123
@Override
116124
public void setStatus(Status status) {
117125
Preconditions.checkNotNull(status, "status");
118-
otelSpan.setStatus(status.isOk() ? StatusCode.OK : StatusCode.ERROR);
126+
DelegatingSpan.super.setStatus(status.isOk() ? StatusCode.OK : StatusCode.ERROR);
119127
}
120128

121129
@Override
122130
public io.opentelemetry.api.trace.Span setStatus(StatusCode canonicalCode, String description) {
123-
return otelSpan.setStatus(canonicalCode, description);
131+
return DelegatingSpan.super.setStatus(canonicalCode, description);
124132
}
125133

126134
@Override
127135
public void end(EndSpanOptions options) {
128-
otelSpan.end();
129-
}
130-
131-
@Override
132-
@SuppressWarnings("ParameterPackage")
133-
public void end(long timestamp, TimeUnit unit) {
134-
otelSpan.end(timestamp, unit);
135-
}
136-
137-
@Override
138-
public <T> io.opentelemetry.api.trace.Span setAttribute(AttributeKey<T> key, @Nonnull T value) {
139-
return otelSpan.setAttribute(key, value);
140-
}
141-
142-
@Override
143-
public io.opentelemetry.api.trace.Span addEvent(String name) {
144-
return otelSpan.addEvent(name);
145-
}
146-
147-
@Override
148-
public io.opentelemetry.api.trace.Span addEvent(String name, long timestamp, TimeUnit unit) {
149-
return otelSpan.addEvent(name, timestamp, unit);
150-
}
151-
152-
@Override
153-
public io.opentelemetry.api.trace.Span addEvent(String name, Attributes attributes) {
154-
return otelSpan.addEvent(name, attributes);
155-
}
156-
157-
@Override
158-
public io.opentelemetry.api.trace.Span addEvent(
159-
String name, Attributes attributes, long timestamp, TimeUnit unit) {
160-
return otelSpan.addEvent(name, attributes, timestamp, unit);
161-
}
162-
163-
@Override
164-
public io.opentelemetry.api.trace.Span recordException(Throwable exception) {
165-
return otelSpan.recordException(exception);
166-
}
167-
168-
@Override
169-
public io.opentelemetry.api.trace.Span recordException(
170-
Throwable exception, Attributes additionalAttributes) {
171-
return otelSpan.recordException(exception, additionalAttributes);
172-
}
173-
174-
@Override
175-
public io.opentelemetry.api.trace.Span updateName(String name) {
176-
return otelSpan.updateName(name);
136+
DelegatingSpan.super.end();
177137
}
178138

179139
@Override
180140
public SpanContext getSpanContext() {
181-
return otelSpan.getSpanContext();
141+
return DelegatingSpan.super.getSpanContext();
182142
}
183143

184144
@Override

0 commit comments

Comments
 (0)