From bafdc31ed4ec0eed4a65b83b2a012ba5a4246194 Mon Sep 17 00:00:00 2001 From: Anton Kolesnikov Date: Mon, 6 Nov 2023 12:30:52 +0800 Subject: [PATCH] Replace `pyroscope.profiling.enabled` attribute with `pyroscope.profile.id` (#64) --- otelpyroscope/README.md | 18 ++++++++---------- otelpyroscope/otelpyroscope.go | 12 ++++++------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/otelpyroscope/README.md b/otelpyroscope/README.md index 211678e..cfa62fb 100644 --- a/otelpyroscope/README.md +++ b/otelpyroscope/README.md @@ -4,19 +4,17 @@ The package provides means to integrate tracing with profiling. More specificall that annotates profiling data with span IDs: when a new trace span emerges, the tracer adds a `span_id` [pprof tag](https://github.com/google/pprof/blob/master/doc/README.md#tag-filtering) that points to the span. This makes it possible to filter out a profile of a particular trace span in [Pyroscope](https://pyroscope.io). -## Example - -You can find a complete example setup in the [Pyroscope repository](https://github.com/grafana/pyroscope/tree/main/examples/tracing/tempo). - -## Other Notes - Note that the module does not control `pprof` profiler itself – it still needs to be started for profiles to be collected. This can be done either via `runtime/pprof` package, or using the [Pyroscope client](https://github.com/grafana/pyroscope-go). By default, only the root span gets labeled (the first span created locally): such spans are marked with the -`pyroscope.profiling.enabled` attribute. Please note that presence of the attribute does not indicate that the -span has a profile: stack trace samples might not be collected, if the actual utilized CPU time is less than the -sample interval (10ms). +`pyroscope.profile.id` attribute set to the span ID. Please note that presence of the attribute does not necessarily +indicate that the span has a profile: stack trace samples might not be collected, if the utilized CPU time is +less than the sample interval (10ms). Limitations: - - Only CPU profiling is fully supported at the moment. +- Only CPU profiling is fully supported at the moment. + +## Example + +You can find a complete example setup in the [Pyroscope repository](https://github.com/grafana/pyroscope/tree/main/examples/tracing/tempo). diff --git a/otelpyroscope/otelpyroscope.go b/otelpyroscope/otelpyroscope.go index d174f7d..d51a709 100644 --- a/otelpyroscope/otelpyroscope.go +++ b/otelpyroscope/otelpyroscope.go @@ -13,7 +13,7 @@ const ( spanNameLabelName = "span_name" ) -var profilingEnabledSpanAttributeKey = attribute.Key("pyroscope.profiling.enabled") +var profileIDSpanAttributeKey = attribute.Key("pyroscope.profile.id") type Option func(*tracerProvider) @@ -97,13 +97,13 @@ func (w *profileTracer) Start(ctx context.Context, spanName string, opts ...trac labels = append(labels, spanIDLabelName, spanID) } - // We mark spans with "pyroscope.profiling.enabled" attribute, - // only if they can have profiles. Note that the presence - // of the attribute does not indicate that we actually have - // collected any samples for the span. + // We mark spans with "pyroscope.profile.id" attribute, + // only if they _can_ have profiles. Presence of the attribute + // does not indicate the fact that we actually have collected + // any samples for the span. if (w.p.config.spanIDScope == scopeRootSpan && spanID == rs.id) || w.p.config.spanIDScope == scopeAllSpans { - span.SetAttributes(profilingEnabledSpanAttributeKey.Bool(true)) + span.SetAttributes(profileIDSpanAttributeKey.String(spanID)) } ctx = pprof.WithLabels(ctx, pprof.Labels(labels...))