Skip to content

Commit

Permalink
[chore] Add note to GroupbyTraceprocessor when working with tailsampl…
Browse files Browse the repository at this point in the history
…ing (open-telemetry#26997)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
I'm proposing a small note in both tailsamplingprocessor and
groupbytrace processor.

In order to perform a tail-based sampling, spans need to be grouped by
`trace_id`.
We have a groupbytrace processor with README describes:
> This processor should be used whenever a processor requires grouped
traces to make decisions,
such as a tail-based sampler or ...

The description is correct since it said "(a customized) tail-based
sampler", not the `tailsamplingprocessor` in this repo. But some people
are still misdirected and think it's necessary to setup groupby
processor before tailsampling processor.

**Link to tracking Issue:** <Issue number if applicable>
N/A

**Testing:** <Describe what testing was performed and which tests were
added.>
N/A

**Documentation:** <Describe the documentation added.>
The PR only change the README of both groupbytrace processor and
tailsampling processor.
  • Loading branch information
jiekun authored and jmsnll committed Nov 12, 2023
1 parent 4b7cbce commit 44dc5ce
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
3 changes: 2 additions & 1 deletion processor/groupbytraceprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ pre-determined amount of time before releasing the trace to the next processor.
The expectation is that, generally, traces will be complete after the given time.

This processor should be used whenever a processor requires grouped traces to make decisions,
such as a tail-based sampler or a per-trace metrics processor.
such as a tail-based sampler or a per-trace metrics processor. Note that [`tailsamplingprocessor`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/tailsamplingprocessor)
also implements a similar mechanism and can be used independently.

The batch processor shouldn't be used before this processor, as this one will
probably undo part (or much) of the work that the batch processor performs. It's
Expand Down
1 change: 1 addition & 0 deletions processor/tailsamplingprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<!-- end autogenerated section -->

The tail sampling processor samples traces based on a set of defined policies. All spans for a given trace MUST be received by the same collector instance for effective sampling decisions.
Before performing sampling, spans will be grouped by `trace_id`. Therefore, the tail sampling processor can be used directly without the need for the [`groupbytraceprocessor`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/groupbytraceprocessor).

This processor must be placed in pipelines after any processors that rely on context, e.g. `k8sattributes`. It reassembles spans into new batches, causing them to lose their original context.

Expand Down

0 comments on commit 44dc5ce

Please sign in to comment.