Skip to content

Commit b3b8e39

Browse files
authored
Remove duplicative trace state passthrough methods (#1490)
1 parent 1f94f65 commit b3b8e39

File tree

6 files changed

+36
-51
lines changed

6 files changed

+36
-51
lines changed

tracing/src/main/java/com/palantir/tracing/DeferredTracer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public DeferredTracer(@Safe String operation, @Safe Map<String, String> metadata
9595
Optional<Trace> maybeTrace = Tracer.copyTrace();
9696
if (maybeTrace.isPresent()) {
9797
Trace trace = maybeTrace.get();
98-
this.traceState = trace.getTraceState();
98+
this.traceState = trace.traceState();
9999
this.isObservable = trace.isObservable();
100100
this.parentSpanId = trace.top().map(OpenSpan::getSpanId).orElse(null);
101101
this.operation = operation;

tracing/src/main/java/com/palantir/tracing/InternalTracers.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public static boolean isSampled(DetachedSpan detachedSpan) {
3333

3434
/** Returns requestId of the provided detachedSpan. */
3535
public static Optional<String> getRequestId(DetachedSpan detachedSpan) {
36-
return Optional.ofNullable(Tracer.getRequestId(detachedSpan));
36+
return Tracer.getRequestId(detachedSpan);
3737
}
3838

3939
/**

tracing/src/main/java/com/palantir/tracing/Trace.java

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.ArrayDeque;
3131
import java.util.Deque;
3232
import java.util.Optional;
33-
import org.jspecify.annotations.Nullable;
3433

3534
/**
3635
* Represents a trace as an ordered list of non-completed spans. Supports adding and removing of spans. This class is
@@ -110,32 +109,8 @@ final OpenSpan startSpan(String operation, SpanType type) {
110109
abstract boolean isObservable();
111110

112111
/** The state of the trace which is stored for each created trace. */
113-
final TraceState getTraceState() {
114-
return this.traceState;
115-
}
116-
117-
/** The globally unique non-empty identifier for this call trace. */
118-
final String getTraceId() {
119-
return traceState.traceId();
120-
}
121-
122-
/**
123-
* The request identifier of this trace, or null if undefined.
124-
* <p>
125-
* The request identifier is an implementation detail of this tracing library. A new identifier is generated
126-
* each time a new trace is created with a SERVER_INCOMING root span. This is a convenience in order to
127-
* distinguish between requests with the same traceId.
128-
*/
129-
@Nullable
130-
final String maybeGetRequestId() {
131-
return traceState.requestId();
132-
}
133-
134-
/**
135-
* The user agent propagated across this trace.
136-
*/
137-
final Optional<String> getForUserAgent() {
138-
return Optional.ofNullable(traceState.forUserAgent());
112+
final TraceState traceState() {
113+
return traceState;
139114
}
140115

141116
/** Returns a copy of this Trace which can be independently mutated. */
@@ -202,12 +177,12 @@ boolean isObservable() {
202177

203178
@Override
204179
Trace deepCopy() {
205-
return new Sampled(new ArrayDeque<>(stack), getTraceState());
180+
return new Sampled(new ArrayDeque<>(stack), traceState());
206181
}
207182

208183
@Override
209184
public String toString() {
210-
return "Trace{stack=" + stack + ", isObservable=true, state=" + getTraceState() + "}";
185+
return "Trace{stack=" + stack + ", isObservable=true, state=" + traceState() + "}";
211186
}
212187
}
213188

@@ -270,7 +245,7 @@ boolean isObservable() {
270245

271246
@Override
272247
Trace deepCopy() {
273-
return new Unsampled(numberOfSpans, getTraceState());
248+
return new Unsampled(numberOfSpans, traceState());
274249
}
275250

276251
/** Internal validation, this should never fail because {@link #pop()} only decrements positive values. */
@@ -283,7 +258,7 @@ private void validateNumberOfSpans() {
283258

284259
@Override
285260
public String toString() {
286-
return "Trace{numberOfSpans=" + numberOfSpans + ", isObservable=false, traceState=" + getTraceState() + "}";
261+
return "Trace{numberOfSpans=" + numberOfSpans + ", isObservable=false, traceState=" + traceState() + "}";
287262
}
288263
}
289264
}

tracing/src/main/java/com/palantir/tracing/Tracer.java

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ public static Optional<TraceMetadata> maybeGetTraceMetadata() {
117117
OpenSpan span = trace.top().orElse(null);
118118

119119
return Optional.of(TraceMetadata.builder()
120-
.traceId(trace.getTraceState().traceId())
120+
.traceId(trace.traceState().traceId())
121121
.spanId(span != null ? span.getSpanId() : Tracers.randomId())
122-
.requestId(Optional.ofNullable(trace.maybeGetRequestId()))
122+
.requestId(Optional.ofNullable(trace.traceState().requestId()))
123123
.build());
124124
}
125125

@@ -327,9 +327,9 @@ static Detached detachInternal() {
327327
if (maybeOpenSpan == null) {
328328
return NopDetached.INSTANCE;
329329
}
330-
return new SampledDetached(trace.getTraceState(), maybeOpenSpan);
330+
return new SampledDetached(trace.traceState(), maybeOpenSpan);
331331
} else {
332-
return new UnsampledDetachedSpan(trace.getTraceState(), Optional.empty());
332+
return new UnsampledDetachedSpan(trace.traceState(), Optional.empty());
333333
}
334334
}
335335

@@ -351,12 +351,12 @@ static TraceState getTraceState() {
351351
return null;
352352
}
353353

354-
return maybeCurrentTrace.getTraceState();
354+
return maybeCurrentTrace.traceState();
355355
}
356356

357357
private static TraceState getTraceState(@Nullable Trace maybeCurrentTrace, SpanType newSpanType) {
358358
if (maybeCurrentTrace != null) {
359-
return maybeCurrentTrace.getTraceState();
359+
return maybeCurrentTrace.traceState();
360360
}
361361
return TraceState.of(Tracers.randomId(), getRequestIdForSpan(newSpanType), Optional.empty());
362362
}
@@ -368,13 +368,12 @@ private static Optional<String> getRequestIdForSpan(SpanType newSpanType) {
368368
return Optional.empty();
369369
}
370370

371-
@Nullable
372-
static String getRequestId(DetachedSpan detachedSpan) {
371+
static Optional<String> getRequestId(DetachedSpan detachedSpan) {
373372
if (detachedSpan instanceof SampledDetachedSpan sampledDetachedSpan) {
374-
return sampledDetachedSpan.traceState.requestId();
373+
return Optional.ofNullable(sampledDetachedSpan.traceState.requestId());
375374
}
376375
if (detachedSpan instanceof UnsampledDetachedSpan unsampledDetachedSpan) {
377-
return unsampledDetachedSpan.traceState.requestId();
376+
return Optional.ofNullable(unsampledDetachedSpan.traceState.requestId());
378377
}
379378
throw new SafeIllegalStateException("Unknown span type", SafeArg.of("detachedSpan", detachedSpan));
380379
}
@@ -598,7 +597,8 @@ public static <T> void fastCompleteSpan(TagTranslator<? super T> tag, T state) {
598597
if (trace != null) {
599598
Optional<OpenSpan> span = popCurrentSpan(trace);
600599
if (trace.isObservable()) {
601-
completeSpanAndNotifyObservers(span, tag, state, trace.getTraceId());
600+
completeSpanAndNotifyObservers(
601+
span, tag, state, trace.traceState().traceId());
602602
}
603603
} else {
604604
logCompletedWithoutStarted();
@@ -637,7 +637,11 @@ public static Optional<Span> completeSpan(@Safe Map<String, String> metadata) {
637637
return Optional.empty();
638638
}
639639
Optional<Span> maybeSpan = popCurrentSpan(trace)
640-
.map(openSpan -> toSpan(openSpan, MapTagTranslator.INSTANCE, metadata, trace.getTraceId()));
640+
.map(openSpan -> toSpan(
641+
openSpan,
642+
MapTagTranslator.INSTANCE,
643+
metadata,
644+
trace.traceState().traceId()));
641645

642646
// Notify subscribers iff trace is observable
643647
if (maybeSpan.isPresent() && trace.isObservable()) {
@@ -772,15 +776,21 @@ public static boolean hasTraceId() {
772776
* Returns the globally unique identifier for this thread's trace.
773777
*/
774778
public static String getTraceId() {
775-
return checkNotNull(currentTrace.get(), "There is no trace").getTraceId();
779+
return checkNotNull(currentTrace.get(), "There is no trace")
780+
.traceState()
781+
.traceId();
776782
}
777783

778784
/**
779785
* Returns the forUserAgent propagated inside the trace.
780786
*/
781787
static Optional<String> getForUserAgent() {
782788
Trace trace = currentTrace.get();
783-
return trace == null ? Optional.empty() : trace.getForUserAgent();
789+
if (trace == null) {
790+
return Optional.empty();
791+
}
792+
793+
return Optional.ofNullable(trace.traceState().forUserAgent());
784794
}
785795

786796
/**
@@ -852,9 +862,9 @@ static void setTrace(Trace trace) {
852862
currentTrace.set(trace);
853863

854864
// Give log appenders access to the trace id and whether the trace is being sampled
855-
MDC.put(Tracers.TRACE_ID_KEY, trace.getTraceId());
865+
MDC.put(Tracers.TRACE_ID_KEY, trace.traceState().traceId());
856866
setTraceSampledMdcIfObservable(trace.isObservable());
857-
setTraceRequestId(trace.maybeGetRequestId());
867+
setTraceRequestId(trace.traceState().requestId());
858868
}
859869

860870
private static void setTraceSampledMdcIfObservable(boolean observable) {

tracing/src/test/java/com/palantir/tracing/TraceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void testToString_doesNotContainTraceLocals() {
5050
Trace trace = Trace.of(true, TraceState.of("traceId", Optional.empty(), Optional.empty()));
5151

5252
TraceLocal<String> traceLocal = TraceLocal.of();
53-
trace.getTraceState().getOrCreateTraceLocals().put(traceLocal, "secret-value");
53+
trace.traceState().getOrCreateTraceLocals().put(traceLocal, "secret-value");
5454

5555
assertThat(trace.toString()).doesNotContain("secret");
5656
}

tracing/src/test/java/com/palantir/tracing/TracerTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ public void testClearAndGetTraceClearsMdc() {
356356
assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isEqualTo(startTrace);
357357

358358
Trace oldTrace = Tracer.getAndClearTrace();
359-
assertThat(oldTrace.getTraceId()).isEqualTo(startTrace);
359+
assertThat(oldTrace.traceState().traceId()).isEqualTo(startTrace);
360360
assertThat(MDC.get(Tracers.TRACE_ID_KEY)).isNull(); // after clearing, it's empty
361361
} finally {
362362
Tracer.fastCompleteSpan();

0 commit comments

Comments
 (0)