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

Hide default context propagators implementation behind interface. #2089

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.DefaultContextPropagators;
import io.opentelemetry.spi.OpenTelemetryFactory;
import io.opentelemetry.spi.metrics.MeterProviderFactory;
import io.opentelemetry.spi.trace.TracerProviderFactory;
Expand Down Expand Up @@ -124,7 +123,7 @@ public Builder toBuilder() {
}

protected static class Builder implements OpenTelemetryBuilder<Builder> {
protected ContextPropagators propagators = DefaultContextPropagators.builder().build();
protected ContextPropagators propagators = ContextPropagators.noop();

protected TracerProvider tracerProvider;
protected MeterProvider meterProvider;
Expand Down
6 changes: 2 additions & 4 deletions api/src/test/java/io/opentelemetry/api/OpenTelemetryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.DefaultContextPropagators;
import io.opentelemetry.spi.metrics.MeterProviderFactory;
import io.opentelemetry.spi.trace.TracerProviderFactory;
import java.io.File;
Expand Down Expand Up @@ -64,7 +63,6 @@ void testDefault() {
.isEqualTo("DefaultMeterProvider");
assertThat(OpenTelemetry.getGlobalMeterProvider())
.isSameAs(OpenTelemetry.getGlobalMeterProvider());
assertThat(OpenTelemetry.getGlobalPropagators()).isInstanceOf(DefaultContextPropagators.class);
assertThat(OpenTelemetry.getGlobalPropagators()).isSameAs(OpenTelemetry.getGlobalPropagators());
}

Expand Down Expand Up @@ -156,14 +154,14 @@ void testMeterNotFound() {

@Test
void testGlobalPropagatorsSet() {
ContextPropagators propagators = DefaultContextPropagators.builder().build();
ContextPropagators propagators = ContextPropagators.noop();
OpenTelemetry.setGlobalPropagators(propagators);
assertThat(OpenTelemetry.getGlobalPropagators()).isEqualTo(propagators);
}

@Test
void testPropagatorsSet() {
ContextPropagators propagators = DefaultContextPropagators.builder().build();
ContextPropagators propagators = ContextPropagators.noop();
OpenTelemetry instance = DefaultOpenTelemetry.builder().build();
instance.setPropagators(propagators);
assertThat(instance.getPropagators()).isEqualTo(propagators);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.context.propagation;

import static java.util.Objects.requireNonNull;

import javax.annotation.concurrent.ThreadSafe;

/**
Expand Down Expand Up @@ -70,6 +72,32 @@
@ThreadSafe
public interface ContextPropagators {

/**
* Returns a {@link ContextPropagators} which can be used to extract and inject context in text
* payloads with the given {@link TextMapPropagator}. Use {@link
* TextMapPropagator#composite(TextMapPropagator...)} to register multiple propagators, which will
* all be executed when extracting or injecting.
*
* <pre>{@code
* ContextPropagators propagators = ContextPropagators.create(
* TextMapPropagator.composite(
* HttpTraceContext.getInstance(),
* W3CBaggagePropagator.getInstance(),
* new MyCustomContextPropagator()));
Copy link
Contributor

Choose a reason for hiding this comment

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

this usage looks really good to me. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I preferred the Builder but hiding the default instance looks great. On that note, TraceMultiPropagator may need to be lifted too, i.e. no Builder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for calling out - let me followup with consistency in TraceMultiPropagator in a bit

* }</pre>
*/
@SuppressWarnings("deprecation")
static ContextPropagators create(TextMapPropagator textPropagator) {
requireNonNull(textPropagator, "textPropagator");
return new DefaultContextPropagators(textPropagator);
}

/** Returns a {@link ContextPropagators} which performs no injection or extraction. */
@SuppressWarnings("deprecation")
static ContextPropagators noop() {
return DefaultContextPropagators.noop();
}

/**
* Returns a {@link TextMapPropagator} propagator.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,8 @@

package io.opentelemetry.context.propagation;

import io.opentelemetry.context.Context;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import javax.annotation.Nullable;

/**
* {@code DefaultContextPropagators} is the default, built-in implementation of {@link
Expand All @@ -21,8 +16,19 @@
* synchronically upon injection and extraction.
*
* <p>The propagation fields retrieved from all registered propagators are de-duplicated.
*
* @deprecated Use {@link ContextPropagators#create(TextMapPropagator)}
*/
@Deprecated
public final class DefaultContextPropagators implements ContextPropagators {

private static final ContextPropagators NOOP =
new DefaultContextPropagators(NoopTextMapPropagator.getInstance());

static ContextPropagators noop() {
return NOOP;
}

private final TextMapPropagator textMapPropagator;

@Override
Expand All @@ -35,45 +41,32 @@ public TextMapPropagator getTextMapPropagator() {
* object.
*
* @return a {@link DefaultContextPropagators.Builder}.
* @deprecated Use {@link ContextPropagators#create(TextMapPropagator)}
*/
@Deprecated
public static Builder builder() {
return new Builder();
}

private DefaultContextPropagators(TextMapPropagator textMapPropagator) {
DefaultContextPropagators(TextMapPropagator textMapPropagator) {
this.textMapPropagator = textMapPropagator;
}

/**
* {@link Builder} is used to construct a new {@code ContextPropagators} object with the specified
* propagators.
* A builder of {@link DefaultContextPropagators}.
*
* <p>Invocation order of {@code TextMapPropagator#inject()} and {@code
* TextMapPropagator#extract()} for registered trace propagators is undefined.
*
* <p>This is a example of a {@code ContextPropagators} object being created:
*
* <pre>{@code
* ContextPropagators propagators = DefaultContextPropagators.builder()
* .addTextMapPropagator(new HttpTraceContext())
* .addTextMapPropagator(new HttpBaggage())
* .addTextMapPropagator(new MyCustomContextPropagator())
* .build();
* }</pre>
* @deprecated Use {@link ContextPropagators#create(TextMapPropagator)}
*/
@Deprecated
public static final class Builder {
List<TextMapPropagator> textPropagators = new ArrayList<>();

/**
* Adds a {@link TextMapPropagator} propagator.
* Add a {@link TextMapPropagator}.
*
* <p>One propagator per concern (traces, correlations, etc) should be added if this format is
* supported.
*
* @param textMapPropagator the propagator to be added.
* @return this.
* @throws NullPointerException if {@code textMapPropagator} is {@code null}.
* @deprecated Use {@link ContextPropagators#create(TextMapPropagator)}
*/
@Deprecated
public Builder addTextMapPropagator(TextMapPropagator textMapPropagator) {
if (textMapPropagator == null) {
throw new NullPointerException("textMapPropagator");
Expand All @@ -84,73 +77,13 @@ public Builder addTextMapPropagator(TextMapPropagator textMapPropagator) {
}

/**
* Builds a new {@code ContextPropagators} with the specified propagators.
* Returns a {@link ContextPropagators}.
*
* @return the newly created {@code ContextPropagators} instance.
* @deprecated Use {@link ContextPropagators#create(TextMapPropagator)}
*/
@Deprecated
public ContextPropagators build() {
if (textPropagators.isEmpty()) {
return new DefaultContextPropagators(NoopTextMapPropagator.INSTANCE);
}

return new DefaultContextPropagators(new MultiTextMapPropagator(textPropagators));
}
}

private static final class MultiTextMapPropagator implements TextMapPropagator {
private final TextMapPropagator[] textPropagators;
private final List<String> allFields;

private MultiTextMapPropagator(List<TextMapPropagator> textPropagators) {
this.textPropagators = new TextMapPropagator[textPropagators.size()];
textPropagators.toArray(this.textPropagators);
this.allFields = Collections.unmodifiableList(getAllFields(this.textPropagators));
}

@Override
public List<String> fields() {
return allFields;
}

private static List<String> getAllFields(TextMapPropagator[] textPropagators) {
Set<String> fields = new LinkedHashSet<>();
for (int i = 0; i < textPropagators.length; i++) {
fields.addAll(textPropagators[i].fields());
}

return new ArrayList<>(fields);
}

@Override
public <C> void inject(Context context, @Nullable C carrier, Setter<C> setter) {
for (int i = 0; i < textPropagators.length; i++) {
textPropagators[i].inject(context, carrier, setter);
}
}

@Override
public <C> Context extract(Context context, @Nullable C carrier, Getter<C> getter) {
for (int i = 0; i < textPropagators.length; i++) {
context = textPropagators[i].extract(context, carrier, getter);
}
return context;
}
}

private static final class NoopTextMapPropagator implements TextMapPropagator {
private static final NoopTextMapPropagator INSTANCE = new NoopTextMapPropagator();

@Override
public List<String> fields() {
return Collections.emptyList();
}

@Override
public <C> void inject(Context context, @Nullable C carrier, Setter<C> setter) {}

@Override
public <C> Context extract(Context context, @Nullable C carrier, Getter<C> getter) {
return context;
return ContextPropagators.create(TextMapPropagator.composite(textPropagators));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.context.propagation;

import io.opentelemetry.context.Context;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import javax.annotation.Nullable;

final class MultiTextMapPropagator implements TextMapPropagator {
private final TextMapPropagator[] textPropagators;
private final List<String> allFields;

MultiTextMapPropagator(List<TextMapPropagator> textPropagators) {
this.textPropagators = new TextMapPropagator[textPropagators.size()];
textPropagators.toArray(this.textPropagators);
this.allFields = Collections.unmodifiableList(getAllFields(this.textPropagators));
}

@Override
public List<String> fields() {
return allFields;
}

private static List<String> getAllFields(TextMapPropagator[] textPropagators) {
Set<String> fields = new LinkedHashSet<>();
for (int i = 0; i < textPropagators.length; i++) {
fields.addAll(textPropagators[i].fields());
}

return new ArrayList<>(fields);
}

@Override
public <C> void inject(Context context, @Nullable C carrier, Setter<C> setter) {
for (int i = 0; i < textPropagators.length; i++) {
textPropagators[i].inject(context, carrier, setter);
}
}

@Override
public <C> Context extract(Context context, @Nullable C carrier, Getter<C> getter) {
for (int i = 0; i < textPropagators.length; i++) {
context = textPropagators[i].extract(context, carrier, getter);
}
return context;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.context.propagation;

import io.opentelemetry.context.Context;
import java.util.Collections;
import java.util.List;
import javax.annotation.Nullable;

final class NoopTextMapPropagator implements TextMapPropagator {
private static final NoopTextMapPropagator INSTANCE = new NoopTextMapPropagator();

static TextMapPropagator getInstance() {
return INSTANCE;
}

@Override
public List<String> fields() {
return Collections.emptyList();
}

@Override
public <C> void inject(Context context, @Nullable C carrier, Setter<C> setter) {}

@Override
public <C> Context extract(Context context, @Nullable C carrier, Getter<C> getter) {
return context;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package io.opentelemetry.context.propagation;

import io.opentelemetry.context.Context;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
Expand Down Expand Up @@ -39,6 +41,39 @@
*/
@ThreadSafe
public interface TextMapPropagator {

/**
* Returns a {@link TextMapPropagator} which simply delegates injection and extraction to the
* provided propagators.
*
* <p>Invocation order of {@code TextMapPropagator#inject()} and {@code
* TextMapPropagator#extract()} for registered trace propagators is undefined.
*/
static TextMapPropagator composite(TextMapPropagator... propagators) {
return composite(Arrays.asList(propagators));
}

/**
* Returns a {@link TextMapPropagator} which simply delegates injection and extraction to the
* provided propagators.
*
* <p>Invocation order of {@code TextMapPropagator#inject()} and {@code
* TextMapPropagator#extract()} for registered trace propagators is undefined.
*/
static TextMapPropagator composite(Iterable<TextMapPropagator> propagators) {
List<TextMapPropagator> propagatorsList = new ArrayList<>();
for (TextMapPropagator propagator : propagators) {
propagatorsList.add(propagator);
}
if (propagatorsList.isEmpty()) {
return NoopTextMapPropagator.getInstance();
}
if (propagatorsList.size() == 1) {
return propagatorsList.get(0);
}
return new MultiTextMapPropagator(propagatorsList);
}

/**
* The propagation fields defined. If your carrier is reused, you should delete the fields here
* before calling {@link #inject(Context, Object, Setter)} )}.
Expand Down
Loading