Skip to content

Commit

Permalink
Consolidates notes on "extra" data (#1424)
Browse files Browse the repository at this point in the history
Before this change, notes about state, particularly the "one instance
per plugin" rule were not consolidated on the method that can cause
problems `addExtra`. This migrates notes together with additional
warnings that when we talk about this being used for propagation
plugins, we also mean not for ad-hoc usage, as that can lead to
excessive overhead.

See #1421

Signed-off-by: Adrian Cole <[email protected]>
  • Loading branch information
codefromthecrypt authored Mar 12, 2024
1 parent d8dc495 commit b7ece7b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 14 deletions.
4 changes: 2 additions & 2 deletions brave/src/main/java/brave/baggage/BaggageFields.java
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 All @@ -25,7 +25,7 @@
* <h3>Built-in fields</h3>
* The following are fields that dispatch to methods on the {@link TraceContext}. They are available
* regardless of {@link BaggagePropagation}. None will return in lookups such as {@link
* BaggageField#getAll(TraceContext)} or {@link BaggageField#getByName(TraceContext, String)}
* BaggageField#getAllValues(TraceContext)} or {@link BaggageField#getByName(TraceContext, String)}.
*
* <p><ol>
* <li>{@link #TRACE_ID}</li>
Expand Down
42 changes: 31 additions & 11 deletions brave/src/main/java/brave/propagation/TraceContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@
package brave.propagation;

import brave.Span;
import brave.baggage.BaggageFields;
import brave.baggage.BaggagePropagation;
import brave.internal.InternalPropagation;
import brave.internal.Nullable;
import brave.internal.Platform;
import brave.internal.baggage.ExtraBaggageContext;
import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -182,27 +185,26 @@ public boolean shared() {
}

/**
* Returns a list of additional data propagated through this trace.
* Used internally by propagation plugins to search for their instance of state regardless of if
* it is in a {@linkplain TraceContext} or {@linkplain TraceContextOrSamplingFlags}.
*
* <p>The contents are intentionally opaque, deferring to {@linkplain Propagation} to define. An
* example implementation could be storing a class containing a correlation value, which is
* extracted from incoming requests and injected as-is onto outgoing requests.
*
* <p>Implementations are responsible for scoping any data stored here. This can be performed
* when {@link Propagation.Factory#decorate(TraceContext)} is called.
* <p>Tools that work only on {@linkplain TraceContext} should use {@link #findExtra(Class)}
* instead.
*
* @see #findExtra(Class)
* @see TraceContextOrSamplingFlags#extra()
* @see Builder#addExtra(Object) for notes on extra values.
* @since 4.9
*/
public List<Object> extra() {
return extraList;
}

/**
* Returns a {@linkplain #extra() propagated state} of the given type if present or null if not.
* Used internally by propagation plugins to search for their instance of state.
*
* <p>Note: It is the responsibility of {@link Propagation.Factory#decorate(TraceContext)}
* to consolidate elements. If it doesn't, there could be multiple instances of a given type and
* this can break logic.
* @see #extra()
* @see Builder#addExtra(Object) for notes on extra values.
*/
@Nullable public <T> T findExtra(Class<T> type) {
return findExtra(type, extraList);
Expand Down Expand Up @@ -372,6 +374,24 @@ public Builder clearExtra() {
}

/**
* This is an advanced function used for {@link Propagation} plugins, such as
* {@link BaggagePropagation}, to add an internal object to hold state when it isn't already in
* {@linkplain #extra()}.
*
* <p>The "extra" parameter is intentionally opaque, and should be an internal type defined by
* a {@linkplain Propagation} plugin. An example implementation could be storing a class
* containing a correlation value, which is extracted from incoming requests and injected as-is
* onto outgoing requests. A real world use is {@link BaggageFields}, which holds all baggage
* fields to propagate in one instance.
*
* <p>Note: It is the responsibility of {@link Propagation.Factory#decorate(TraceContext)}
* to consolidate elements (by searching for an existing instance of their state with {@link
* #findExtra(Class)} or {@link #extra()}). If it doesn't, there could be multiple instances of
* a given type and this can break logic and add overhead. Decoration is also when
* implementations define their scope. For example, a plugin that only propagates a field
* without changing will be constant for the whole request. A plugin that allows changes of
* state need careful coding, such as done in {@link BaggagePropagation}.
*
* @see #extra()
* @since 5.12
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package brave.propagation;

import brave.Tracer;
import brave.baggage.BaggagePropagation;
import brave.internal.InternalPropagation;
import brave.internal.Nullable;
import brave.propagation.TraceContext.Extractor;
Expand Down Expand Up @@ -245,9 +246,10 @@ public TraceContextOrSamplingFlags sampled(boolean sampled) {
* #context()} is {@code null}.
*
* @see TraceContext#extra()
* @see Builder#addExtra(Object) for notes on extra values.
* @since 4.9
*/
public final List<Object> extra() {
public List<Object> extra() {
return extraList;
}

Expand Down Expand Up @@ -309,7 +311,16 @@ public Builder sampledLocal() {
}

/**
* This is an advanced function used for {@link Propagation} plugins, such as
* {@link BaggagePropagation}, to add an internal object to hold state before extracting a
* remote request.
*
* <p>Implications of data are the same as {@link TraceContext.Builder#addExtra(Object)}. The
* main difference here is that {@link Extractor#extract(Object)} may not result in a trace
* context. For example, baggage fields can exist without an incoming trace.
*
* @see TraceContextOrSamplingFlags#extra()
* @see TraceContext.Builder#addExtra(Object)
* @since 4.9
*/
public Builder addExtra(Object extra) {
Expand Down

0 comments on commit b7ece7b

Please sign in to comment.