Implement config to ignore the sampled flag in traceparent #1310

gregkalapos · 2021-05-26T17:32:33Z

The problem

We started seeing environments where the full trace is not within elastic and the sample rate is effectively 0% regardless of the TransactionSampleRate setting. Very likely this is because unmonitored .NET5+ services send a traceparent header with a sampled flag set to 0 (doc on the change from Microsoft)

Proposed solution in this PR

We add a new setting called SuppressTraceContextHeaders. If this is true we ignore any distributed tracing data during transaction start and we make a new sampling decision regardless of the traceparent value. Update: see comments below, the solution evolved during the PR reviews.

apmmachine · 2021-05-26T18:19:30Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Branch indexing
Start Time: 2021-06-14T13:34:41.472+0000
Duration: 48 min 19 sec
Commit: 3ce68a6

Test stats 🧪

Test	Results
Failed	0
Passed	19429
Skipped	101
Total	19530

Trends 🧪

gregkalapos · 2021-05-27T08:09:08Z

Alternatives:

make the setting into a URL pattern (set URLs where you ignore the headers and apply it for the rest)
only apply when tracestate=es:... is NOT present (aka the request is not from our agent)

Maybe make it TraceContextHeadersIgnore.

See if traceparent or tracestate has any indication if it was set from Activity.

russcam · 2021-06-04T04:26:24Z

@gregkalapos, @AlexanderWert Another alternative, which is along similar lines to ones proposed:

New configuration value: TraceContextIgnoreSampledFalse: true|false, with a default value of false.

If TraceContextIgnoreSampledFalse is set to true, when a transaction is started, if there is Trace Context:

If the sampled flag is 1, proceed as per current implementation
If the sampled flag is 0, check the tracestate
- If tracestate contains es vendor and s value, proceed as per current implementation
- If tracestate does not contain es vendor and s value
  1. determine whether the transaction should be sampled based on the transaction id and set IsSampled
  2. set the sample rate from the sampler
  3. use the trace id, parent id of Trace Context
  4. mutate tracestate of Trace Context to add es vendor and s value to and use

The end result is that sampled flag value of 0 is ignored when tracestate does not contain es vendor and s value

gregkalapos · 2021-06-06T18:27:05Z

@gregkalapos, @AlexanderWert Another alternative, which is along similar lines to ones proposed:

New configuration value: TraceContextIgnoreSampledFalse: true|false, with a default value of false.

If TraceContextIgnoreSampledFalse is set to true, when a transaction is started, if there is Trace Context:

If the sampled flag is 1, proceed as per current implementation

If the sampled flag is 0, check the tracestate

If tracestate contains es vendor and s value, proceed as per current implementation

If tracestate does not contain es vendor and s value

determine whether the transaction should be sampled based on the transaction id and set IsSampled

set the sample rate from the sampler

use the trace id, parent id of Trace Context

mutate tracestate of Trace Context to add es vendor and s value to and use

The end result is that sampled flag value of 0 is ignored when tracestate does not contain es vendor and s value

@russcam the suggestion sounds good to me.

The advantage is clearly that this would solve the issues we have with .NET5+ unmonitored services while it causes the least harm in other environments.

The only disadvantage I see is that people will have a super hard time to understand this - I don't expect users to understand the W3C TraceContext spec, so it'll be super hard to explain in the docs what this config exactly does.

Overall I think this is a good way to proceed and worth accepting the more complicated setting - I'll update the PR.

gregkalapos · 2021-06-09T21:31:04Z

Pushed the changes discussed above - ready for another review round.

russcam

I've had an initial look and left some comments

russcam · 2021-06-14T04:49:42Z

src/Elastic.Apm/Config/ConfigConsts.cs

@@ -32,6 +32,7 @@ public static class DefaultValues
 			public const string SpanFramesMinDuration = "5ms";
 			public const double SpanFramesMinDurationInMilliseconds = 5;
 			public const int StackTraceLimit = 50;
+			public const bool SuppressTraceContextHeaders = false;


Suggested change

public const bool SuppressTraceContextHeaders = false;

public const bool TraceContextIgnoreSampledFalse = false;

Consistent name for the default value constant

👍 Fixed this and the usage of it as well.

russcam · 2021-06-14T05:00:09Z

docs/configuration.asciidoc

+[[config-trace-context-ignore-sampled-false]]
+==== `TraceContextIgnoreSampledFalse`
+
+By setting this to `true` the agent will ignore the sampled part of the W3C TraceContext traceparent header when it's false and when the upstream transaction is not coming from a non-Elastic APM agent.


It might be useful to explain the "normal" flow when Trace Context is received, and then lead into describing what this setting does. In doing so, I think it would provide more context around why it is needed and if it might be relevant to a specific user.

docs/configuration.asciidoc

russcam · 2021-06-14T05:18:31Z

docs/configuration.asciidoc

+This can be useful when a caller service always sets the sampled flag to false and the agent would have no chance to
+create any sampled transaction.
+
+NOTE: .NET 5 by default sets the W3C TraceContext headers, but without an active agent it sets the sampled flag to `false` leading to 0% sampling. If your service is called by an unmonitored .NET 5 application, setting this flag will instruct the agent to start a new trace and make a new sampling decision based on <<config-transaction-sample-rate>>.


Suggested change

NOTE: .NET 5 by default sets the W3C TraceContext headers, but without an active agent it sets the sampled flag to `false` leading to 0% sampling. If your service is called by an unmonitored .NET 5 application, setting this flag will instruct the agent to start a new trace and make a new sampling decision based on <<config-transaction-sample-rate>>.

[IMPORTANT]

--

.NET 5 applications set the https://www.w3.org/TR/trace-context/[W3C Trace Context] headers for outgoing HTTP requests by default, but with the traceparent sampled flag set to `false`. If the application has an active agent, the agent will ensure that the sampled flag is propagated appropriately. If the application does not have an active agent however, and the application calls another application (the callee) that does have an active agent, the propagation of a sampled flag of `false` results in no sampled traces in the callee application.

If your application is called by an unmonitored .NET 5 application, setting `TraceContextIgnoreSampledFalse` to `true` will instruct the agent to start a new trace and make a new sampling decision based on <<config-transaction-sample-rate>> when the traceparent sampled flag is `false` and there is no agent specific tracestate present.

--

Maybe the section should lead with this?

Sounds good, moved this to the top of the section.

gregkalapos · 2021-06-14T10:44:42Z

docs/configuration.asciidoc

+
+[IMPORTANT]
+--
+..NET 5 applications set the https://www.w3.org/TR/trace-context/[W3C Trace Context] headers for outgoing HTTP requests by default, but with the traceparent sampled flag set to `false`. If the application has an active agent, the agent will ensure that the sampled flag is propagated appropriately. If the application does not have an active agent however, and the application calls another application (the callee) that does have an active agent, the propagation of a sampled flag of `false` results in no sampled traces in the callee application.


It seems we need an additional . to render .NET in asciidoc when it's the 1. word in the section.

This looks like it renders the paragraph in red italics in asciidoctor.js

An attribute entry can be used

Suggested change

..NET 5 applications set the https://www.w3.org/TR/trace-context/[W3C Trace Context] headers for outgoing HTTP requests by default, but with the traceparent sampled flag set to `false`. If the application has an active agent, the agent will ensure that the sampled flag is propagated appropriately. If the application does not have an active agent however, and the application calls another application (the callee) that does have an active agent, the propagation of a sampled flag of `false` results in no sampled traces in the callee application.

:dot: .

{dot}NET 5 applications set the https://www.w3.org/TR/trace-context/[W3C Trace Context] headers for outgoing HTTP requests by default, but with the traceparent sampled flag set to `false`. If the application has an active agent, the agent will ensure that the sampled flag is propagated appropriately. If the application does not have an active agent however, and the application calls another application (the callee) that does have an active agent, the propagation of a sampled flag of `false` results in no sampled traces in the callee application.

which renders as

Can ignore the suggestion here as it's included in the next suggestion

russcam

LGTM. I've left a suggestion for the docs

docs/configuration.asciidoc

russcam · 2021-06-15T00:45:40Z

docs/configuration.asciidoc

+
+[IMPORTANT]
+--
+..NET 5 applications set the https://www.w3.org/TR/trace-context/[W3C Trace Context] headers for outgoing HTTP requests by default, but with the traceparent sampled flag set to `false`. If the application has an active agent, the agent will ensure that the sampled flag is propagated appropriately. If the application does not have an active agent however, and the application calls another application (the callee) that does have an active agent, the propagation of a sampled flag of `false` results in no sampled traces in the callee application.


Can ignore the suggestion here as it's included in the next suggestion

Turned SuppressTraceContextHeaders into TraceContextIgnoreSampledFalse.

Co-authored-by: Russ Cam <[email protected]>

github-actions bot added the agent-dotnet label May 26, 2021

gregkalapos self-assigned this May 26, 2021

gregkalapos requested a review from russcam May 26, 2021 18:24

gregkalapos added the v1.11.0 label May 27, 2021

russcam reviewed Jun 14, 2021

View reviewed changes

gregkalapos commented Jun 14, 2021

View reviewed changes

gregkalapos changed the title ~~Implement SuppressTraceContextHeaders config to ignore W3C headers~~ Implement config to ignore the sampled flag in traceparent Jun 14, 2021

russcam approved these changes Jun 15, 2021

View reviewed changes

gregkalapos and others added 5 commits June 15, 2021 20:29

Implement SuppressTraceContextHeaders config to ignore W3C headers

36cda7a

Implement TraceContextIgnoreSampledFalse config

02f3be8

Turned SuppressTraceContextHeaders into TraceContextIgnoreSampledFalse.

Review feedback

84ed659

Update DistributedTracingAspNetCoreTests.cs

040ef0e

Update docs/configuration.asciidoc

4189900

Co-authored-by: Russ Cam <[email protected]>

gregkalapos merged commit 708ece0 into elastic:master Jun 15, 2021

gregkalapos deleted the SuppressTraceparent branch June 15, 2021 18:31

russcam added the enhancement New feature or request label Jun 16, 2021

gregkalapos mentioned this pull request Jul 28, 2021

Unset parentId if TraceContextIgnoreSampledFalse is active #1362

Merged

gregkalapos mentioned this pull request Aug 30, 2021

Traceparent header from outside Elastic elastic/apm#286

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement config to ignore the sampled flag in traceparent #1310

Implement config to ignore the sampled flag in traceparent #1310

gregkalapos commented May 26, 2021 •

edited

Loading

apmmachine commented May 26, 2021 •

edited

Loading

Build stats

Test stats 🧪

Trends 🧪

gregkalapos commented May 27, 2021 •

edited

Loading

russcam commented Jun 4, 2021

gregkalapos commented Jun 6, 2021

gregkalapos commented Jun 9, 2021

russcam left a comment

russcam Jun 14, 2021

gregkalapos Jun 14, 2021

russcam Jun 14, 2021

gregkalapos Jun 14, 2021

russcam Jun 14, 2021

gregkalapos Jun 14, 2021

gregkalapos Jun 14, 2021

russcam Jun 15, 2021

russcam Jun 15, 2021

russcam left a comment

russcam Jun 15, 2021

	public const bool SuppressTraceContextHeaders = false;
	public const bool TraceContextIgnoreSampledFalse = false;

Implement config to ignore the sampled flag in traceparent #1310

Implement config to ignore the sampled flag in traceparent #1310

Conversation

gregkalapos commented May 26, 2021 • edited Loading

The problem

Proposed solution in this PR

apmmachine commented May 26, 2021 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

gregkalapos commented May 27, 2021 • edited Loading

russcam commented Jun 4, 2021

gregkalapos commented Jun 6, 2021

gregkalapos commented Jun 9, 2021

russcam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

russcam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregkalapos commented May 26, 2021 •

edited

Loading

apmmachine commented May 26, 2021 •

edited

Loading

gregkalapos commented May 27, 2021 •

edited

Loading