Skip to content

Commit

Permalink
Undeprecates propagation symbols for v6 interop
Browse files Browse the repository at this point in the history
This undeprecates B3SinglePropagation and enforces at runtime that
`Propagation.Factory#get()` must be implemented. I spot checked all the
implementations on GitHub already do.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
Adrian Cole committed Jan 7, 2024
1 parent 1de18ef commit fa43521
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 74 deletions.
7 changes: 1 addition & 6 deletions brave/src/main/java/brave/baggage/BaggagePropagation.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -18,7 +18,6 @@
import brave.internal.baggage.BaggageCodec;
import brave.internal.baggage.BaggageFields;
import brave.internal.collect.Lists;
import brave.internal.propagation.StringPropagationAdapter;
import brave.propagation.ExtraFieldPropagation;
import brave.propagation.Propagation;
import brave.propagation.TraceContext;
Expand Down Expand Up @@ -198,10 +197,6 @@ static final class Factory extends Propagation.Factory implements Propagation<St
this.localFieldNames = localFieldNames.toArray(new String[0]);
}

@Deprecated @Override public <K1> BaggagePropagation<K1> create(KeyFactory<K1> keyFactory) {
return new BaggagePropagation<K1>(StringPropagationAdapter.create(get(), keyFactory));
}

@Override public BaggagePropagation<String> get() {
return new BaggagePropagation<String>(this);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -70,7 +70,10 @@
* }</pre>
*
* @since 5.12
* @deprecated As of Brave 5.18, throw an {@link UnsupportedOperationException} in
* {@link #create(Propagation, KeyFactory)} instead of implementing it.
*/
@Deprecated
public final class StringPropagationAdapter<K> implements Propagation<K> {
public static <K> Propagation<K> create(Propagation<String> delegate, KeyFactory<K> keyFactory) {
if (delegate == null) throw new NullPointerException("delegate == null");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2019 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -18,9 +18,9 @@
/**
* Implements the propagation format described in {@link B3SingleFormat}.
*
* @deprecated Since 5.9, use {@link B3Propagation#newFactoryBuilder()} to control inject formats.
* <p>Use {@link B3Propagation#newFactoryBuilder()} to control inject formats.
*/
@Deprecated public final class B3SinglePropagation {
public final class B3SinglePropagation {
public static final Propagation.Factory FACTORY = B3Propagation.newFactoryBuilder()
.injectFormat(B3Propagation.Format.SINGLE)
.injectFormat(Span.Kind.CLIENT, B3Propagation.Format.SINGLE)
Expand Down
17 changes: 4 additions & 13 deletions brave/src/main/java/brave/propagation/ExtraFieldPropagation.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -19,7 +19,6 @@
import brave.internal.Nullable;
import brave.propagation.TraceContext.Extractor;
import brave.propagation.TraceContext.Injector;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -190,24 +189,16 @@ public Factory build() {
/** @deprecated Since 5.11 use {@link Propagation.Factory} */
public static class Factory extends Propagation.Factory {
final Propagation.Factory delegate;
final String[] extraKeyNames;
final List<String> extraKeyNames;

Factory(Propagation.Factory delegate, String[] extraKeyNames) {
this.delegate = delegate;
this.extraKeyNames = extraKeyNames;
this.extraKeyNames = unmodifiableList(Arrays.asList(extraKeyNames));
}

/** {@inheritDoc} */
@Override public ExtraFieldPropagation<String> get() {
return create(KeyFactory.STRING);
}

/** {@inheritDoc} */
@Deprecated @Override
public <K> ExtraFieldPropagation<K> create(Propagation.KeyFactory<K> keyFactory) {
List<K> extraKeys = new ArrayList<K>();
for (String extraKeyName : extraKeyNames) extraKeys.add(keyFactory.create(extraKeyName));
return new ExtraFieldPropagation<K>(delegate.create(keyFactory), unmodifiableList(extraKeys));
return new ExtraFieldPropagation<String>(delegate.get(), extraKeyNames);
}

@Override public boolean supportsJoin() {
Expand Down
51 changes: 25 additions & 26 deletions brave/src/main/java/brave/propagation/Propagation.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -17,7 +17,6 @@
import brave.Span.Kind;
import brave.baggage.BaggagePropagation;
import brave.internal.Nullable;
import brave.internal.propagation.StringPropagationAdapter;
import java.util.List;

/**
Expand All @@ -32,9 +31,11 @@
* request is sent to the server. The server {@linkplain TraceContext.Extractor#extract extracts} a
* trace context from these headers before processing the request.
*
* @param <K> Deprecated except when a {@link String}.
* @param <K> Retained for compatibility with pre Brave 6.0, but always String.
* @since 4.0
*/
// The generic type parameter K is always <String>. Even if the deprecated methods are removed in
// Brave 6.0. This is to avoid a compilation break and revlock.
public interface Propagation<K> {
/**
* Defaults B3 formats based on {@link Request} type. When not a {@link Request} (e.g. in-process
Expand All @@ -44,17 +45,17 @@ public interface Propagation<K> {
*/
Propagation<String> B3_STRING = B3Propagation.get();
/**
* @deprecated Since 5.9, use {@link B3Propagation#newFactoryBuilder()} to control inject formats.
* Implements the propagation format described in {@link B3SingleFormat}.
*/
@Deprecated Propagation<String> B3_SINGLE_STRING = B3SinglePropagation.FACTORY.get();
Propagation<String> B3_SINGLE_STRING = B3SinglePropagation.FACTORY.get();

/** @since 4.0 */
abstract class Factory {
/**
* Does the propagation implementation support sharing client and server span IDs. For example,
* should an RPC server span share the same identifiers extracted from an incoming request?
*
* In usual <a href="https://github.com/openzipkin/b3-propagation">B3 Propagation</a>, the
* <p>In usual <a href="https://github.com/openzipkin/b3-propagation">B3 Propagation</a>, the
* parent span ID is sent across the wire so that the client and server can share the same
* identifiers. Other propagation formats, like <a href="https://github.com/w3c/trace-context">trace-context</a>
* only propagate the calling trace and span ID, with an assumption that the receiver always
Expand All @@ -78,31 +79,28 @@ public boolean requires128BitTraceId() {
}

/**
* This is deprecated: end users and instrumentation should never call this, and instead use
* {@link #get()}.
*
* <h3>Implementation advice</h3>
* This is deprecated, but abstract. This means those implementing custom propagation formats
* will have to implement this until it is removed in Brave 6. If you are able to use a tool
* such as "maven-shade-plugin", consider using {@link StringPropagationAdapter}.
*
* @param <K> Deprecated except when a {@link String}.
* @see KeyFactory#STRING
* @since 4.0
* @deprecated Since 5.12, use {@link #get()}
* @deprecated end users and instrumentation should never call this, and instead use
* {@link #get()}. This will not be removed, to avoid rev-lock upgrading to Brave 6.
*/
@Deprecated
public abstract <K> Propagation<K> create(KeyFactory<K> keyFactory);
@Deprecated public <K> Propagation<K> create(KeyFactory<K> unused) {
// In Brave 5.12, this was abstract, but not used: `get()` dispatched
// to this. Brave 5.18 implemented this with the below exception to force
// `get()` to be overridden. Doing so allows us to make `get()` abstract
// in Brave 6.0, but we will have to leave this here regardless, to
// prevent revlock upgrading.
throw new UnsupportedOperationException("This was replaced with PropagationFactory.get() in Brave 5.12");
}

/**
* Returns a possibly cached propagation instance.
*
* <p>Implementations should override and implement this method directly.
*
* @since 5.12
*/
public Propagation<String> get() {
return create(KeyFactory.STRING);
// In Brave 5.12, this dispatched to the deprecated abstract method
// `create()`. In Brave 5.18, we throw an exception instead to ensure it
// is implemented prior to Brave 6.0 making this abstract.
throw new UnsupportedOperationException("As of Brave 5.18, you must implement PropagationFactory.get()");
}

/**
Expand All @@ -126,7 +124,8 @@ public TraceContext decorate(TraceContext context) {

/**
* @since 4.0
* @deprecated since 5.12 non-string keys are no longer supported
* @deprecated since 5.12 non-string keys are no longer supported. This will not be removed, to
* avoid rev-lock upgrading to Brave 6.
*/
@Deprecated
interface KeyFactory<K> {
Expand All @@ -147,7 +146,7 @@ interface KeyFactory<K> {
* Replaces a propagated key with the given value.
*
* @param <R> Usually, but not always, an instance of {@link Request}.
* @param <K> Deprecated except when a {@link String}.
* @param <K> Retained for compatibility with pre Brave 6.0, but always String.
* @see RemoteSetter
* @since 4.0
*/
Expand Down Expand Up @@ -212,7 +211,7 @@ interface Setter<R, K> {
* Gets the first value of the given propagation key or returns {@code null}.
*
* @param <R> Usually, but not always, an instance of {@link Request}.
* @param <K> Deprecated except when a {@link String}.
* @param <K> Retained for compatibility with pre Brave 6.0, but always String.
* @see RemoteGetter
* @since 4.0
*/
Expand Down
14 changes: 5 additions & 9 deletions brave/src/test/java/brave/TracerTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -19,8 +19,6 @@
import brave.baggage.BaggagePropagationConfig.SingleBaggageField;
import brave.handler.MutableSpan;
import brave.handler.SpanHandler;
import brave.internal.Platform;
import brave.internal.handler.OrphanTracker;
import brave.propagation.B3Propagation;
import brave.propagation.CurrentTraceContext;
import brave.propagation.CurrentTraceContext.Scope;
Expand All @@ -42,7 +40,6 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import zipkin2.Endpoint;
import zipkin2.reporter.Reporter;

Expand All @@ -68,8 +65,8 @@ public class TracerTest {
.addSpanHandler(spans)
.trackOrphans()
.propagationFactory(new Propagation.Factory() {
@Deprecated @Override public <K> Propagation<K> create(Propagation.KeyFactory<K> keyFactory) {
return propagationFactory.create(keyFactory);
@Override public Propagation<String> get() {
return propagationFactory.get();
}

@Override public boolean supportsJoin() {
Expand Down Expand Up @@ -229,9 +226,8 @@ public class TracerTest {
@Test void join_createsChildWhenUnsupportedByPropagation() {
tracer = Tracing.newBuilder()
.propagationFactory(new Propagation.Factory() {
@Override
@Deprecated public <K> Propagation<K> create(Propagation.KeyFactory<K> keyFactory) {
return B3Propagation.FACTORY.create(keyFactory);
@Override public Propagation<String> get() {
return B3Propagation.FACTORY.get();
}
})
.addSpanHandler(spans).build().tracer();
Expand Down
6 changes: 3 additions & 3 deletions brave/src/test/java/brave/TracingTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -243,8 +243,8 @@ class TracingTest {
AtomicBoolean sampledLocal = new AtomicBoolean();
try (Tracing tracing = Tracing.newBuilder()
.propagationFactory(new Propagation.Factory() {
@Deprecated public <K> Propagation<K> create(Propagation.KeyFactory<K> keyFactory) {
return B3SinglePropagation.FACTORY.create(keyFactory);
@Override public Propagation<String> get() {
return B3SinglePropagation.FACTORY.get();
}

@Override public TraceContext decorate(TraceContext context) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2020 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -41,7 +41,7 @@ public final class CustomTraceIdPropagation extends Propagation.Factory
public static Propagation.Factory create(Propagation.Factory delegate, String customTraceIdName) {
if (delegate == null) throw new NullPointerException("delegate == null");
if (customTraceIdName == null) throw new NullPointerException("customTraceIdName == null");
if (!delegate.create(KeyFactory.STRING).keys().contains("b3")) {
if (!delegate.get().keys().contains("b3")) {
throw new IllegalArgumentException("delegate must implement B3 propagation");
}
return new CustomTraceIdPropagation(delegate, customTraceIdName);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -24,14 +24,15 @@
/** This shows propagation keys don't need to be Strings. For example, we can propagate over gRPC */
class NonStringPropagationKeysTest {
TraceContext context = TraceContext.newBuilder().traceId(1).spanId(2).sampled(true).build();

// Note: This class will be removed in Brave 6.0, but B3Propagation implements create meanwhile.
Propagation<Metadata.Key<String>> grpcPropagation = B3Propagation.FACTORY.create(
name -> Metadata.Key.of(name, Metadata.ASCII_STRING_MARSHALLER)
);
TraceContext.Extractor<Metadata> extractor = grpcPropagation.extractor(Metadata::get);
TraceContext.Injector<Metadata> injector = grpcPropagation.injector(Metadata::put);

@Test void injectExtractTraceContext() {

Metadata metadata = new Metadata();
injector.inject(context, metadata);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -30,8 +30,8 @@ class ExtraFactoryTest {
.build();

Propagation.Factory propagationFactory = new Propagation.Factory() {
@Deprecated @Override public <K> Propagation<K> create(Propagation.KeyFactory<K> keyFactory) {
return B3Propagation.FACTORY.create(keyFactory);
@Override public Propagation<String> get() {
return B3Propagation.FACTORY.get();
}

@Override public TraceContext decorate(TraceContext context) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2013-2023 The OpenZipkin Authors
* Copyright 2013-2024 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -16,7 +16,6 @@
import brave.Span.Kind;
import brave.handler.MutableSpan;
import brave.internal.codec.HexCodec;
import brave.internal.propagation.StringPropagationAdapter;
import brave.messaging.MessagingRuleSampler;
import brave.messaging.MessagingTracing;
import brave.propagation.Propagation;
Expand Down Expand Up @@ -213,10 +212,6 @@ static class TraceIdOnlyPropagation extends Propagation.Factory implements Propa
@Override public Propagation<String> get() {
return this;
}

@Override public <K1> Propagation<K1> create(KeyFactory<K1> keyFactory) {
return StringPropagationAdapter.create(this, keyFactory);
}
}

@Test void continues_a_trace_when_only_trace_id_propagated() {
Expand Down

0 comments on commit fa43521

Please sign in to comment.