Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undeprecates propagation symbols for v6 interop #1396

Merged
merged 1 commit into from
Jan 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is internal, but I put a deprecated message on it anyway as many have been using it without shading. The pull requests I mentioned move off it.

* {@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