diff --git a/docs/metrics.md b/docs/metrics.md index 446ad89a..f1e92b3e 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -19,12 +19,13 @@ The Relay Proxy can export metrics via [OpenTelemetry Protocol (OTLP)](https://o ## Attributes -All metrics include the following attributes: +All metrics include the following attributes. When OpenTelemetry tracing is enabled, `environment.name` and `environment.id` are also set on the request span of every authenticated request, using these same keys, so traces and metrics can be correlated by environment. | Attribute | Description | |-----------|-------------| | `relay.id` | A unique identifier for this Relay Proxy instance, generated at startup. | -| `environment.name` | The name of the LaunchDarkly environment as configured in the Relay Proxy. In automatic configuration or offline mode, this is the actual project and environment name from LaunchDarkly. Example: `MyApplication Staging` | +| `environment.name` | The name of the LaunchDarkly environment as configured in the Relay Proxy. In automatic configuration or offline mode, this is the actual project and environment name from LaunchDarkly. If the environment is renamed there, subsequent metrics are reported under the new name, which starts a new series. Example: `MyApplication Staging` | +| `environment.id` | The LaunchDarkly environment ID, when the environment is configured with a client-side environment ID. It is omitted for environments configured with only an SDK key or mobile key. Environments that differ only by payload filter share one environment ID, so group by `environment.name` to tell filtered variants apart. Example: `507f1f77bcf86cd79943902a` | | `platform.category` | The kind of SDK that generated the metric: `server`, `mobile`, or `browser`. | | `user_agent` | The user agent of the SDK making the request. Example: `Node/3.4.0` | | `sdk.wrapper` | The SDK wrapper identifier, if provided. Example: `flutter-client/2.0.0` | diff --git a/internal/metrics/constants.go b/internal/metrics/constants.go index 9c62bd9c..2978f4c3 100644 --- a/internal/metrics/constants.go +++ b/internal/metrics/constants.go @@ -4,6 +4,8 @@ import ( "strings" "time" + "github.com/launchdarkly/ld-relay/v9/internal/tracing" + "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) @@ -26,12 +28,18 @@ const ( ServerPlatformCategory = "server" ) +// The environment attribute keys are shared with the tracing package, which sets the same two +// attributes on the request span. +const ( + envNameAttrKey = tracing.EnvNameKey + envIDAttrKey = tracing.EnvIDKey +) + var ( relayIDAttrKey = attribute.Key("relay.id") //nolint:gochecknoglobals platformCategoryAttrKey = attribute.Key("platform.category") //nolint:gochecknoglobals userAgentAttrKey = attribute.Key("user_agent") //nolint:gochecknoglobals sdkWrapperAttrKey = attribute.Key("sdk.wrapper") //nolint:gochecknoglobals - envNameAttrKey = attribute.Key("environment.name") //nolint:gochecknoglobals applicationIDAttrKey = attribute.Key("application.id") //nolint:gochecknoglobals applicationVersionAttrKey = attribute.Key("application.version") //nolint:gochecknoglobals instanceIDAttrKey = attribute.Key("instance.id") //nolint:gochecknoglobals @@ -47,7 +55,8 @@ var ( ) // buildRequestAttributes creates an OTel attribute set for request metrics using semconv attribute names -// where applicable. All string values should be pre-sanitized via sanitizeTagValue before calling this function. +// where applicable. All string values should be pre-sanitized via tracing.SanitizeAttributeValue before +// calling this function. func buildRequestAttributes(baseKVs []attribute.KeyValue, platform, userAgent, sdkWrapper, route, method, urlScheme, applicationID, applicationVersion, instanceID string) attribute.Set { attrs := make([]attribute.KeyValue, len(baseKVs), len(baseKVs)+9) copy(attrs, baseKVs) @@ -93,18 +102,8 @@ func buildDurationAttributes(baseKVs []attribute.KeyValue, platform, userAgent, return attribute.NewSet(attrs...) } -// sanitizeTagValue ensures attribute values are valid. -// Empty values are replaced with descriptive defaults, and slashes are replaced with underscores. -// This is appropriate for user agent strings and SDK wrapper names, but not for routes. -func sanitizeTagValue(v string) string { - if strings.TrimSpace(v) == "" { - return "not-provided" - } - return strings.ReplaceAll(v, "/", "_") -} - // sanitizeRouteValue ensures route attribute values are valid. -// Empty values are replaced with descriptive defaults. Unlike sanitizeTagValue, +// Empty values are replaced with descriptive defaults. Unlike tracing.SanitizeAttributeValue, // slashes are preserved since they are meaningful in route paths. func sanitizeRouteValue(v string) string { if strings.TrimSpace(v) == "" { diff --git a/internal/metrics/measures.go b/internal/metrics/measures.go index ffcb03b0..bf58e980 100644 --- a/internal/metrics/measures.go +++ b/internal/metrics/measures.go @@ -4,6 +4,8 @@ import ( "context" "time" + "github.com/launchdarkly/ld-relay/v9/internal/tracing" + ldevents "github.com/launchdarkly/go-sdk-events/v3" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -125,13 +127,13 @@ type RequestInfo struct { } func (ri RequestInfo) sanitized() (ua, wrapper, route, method, appID, appVersion, instanceID string) { - return sanitizeTagValue(ri.UserAgent), - sanitizeTagValue(ri.SDKWrapper), + return tracing.SanitizeAttributeValue(ri.UserAgent), + tracing.SanitizeAttributeValue(ri.SDKWrapper), sanitizeRouteValue(ri.Route), - sanitizeTagValue(ri.Method), - sanitizeTagValue(ri.ApplicationID), - sanitizeTagValue(ri.ApplicationVersion), - sanitizeTagValue(ri.InstanceID) + tracing.SanitizeAttributeValue(ri.Method), + tracing.SanitizeAttributeValue(ri.ApplicationID), + tracing.SanitizeAttributeValue(ri.ApplicationVersion), + tracing.SanitizeAttributeValue(ri.InstanceID) } // WithGauge increments the specified metric before running the function and then decrements it (for use with @@ -143,7 +145,7 @@ func WithGauge(em *EnvironmentManager, instruments *Instruments, ri RequestInfo, } ua, wrapper, route, method, appID, appVersion, instanceID := ri.sanitized() - attrs := buildRequestAttributes(em.envKVs, measure.platformCategory, ua, wrapper, route, method, ri.URLScheme, appID, appVersion, instanceID) + attrs := buildRequestAttributes(em.attributes().kvs, measure.platformCategory, ua, wrapper, route, method, ri.URLScheme, appID, appVersion, instanceID) if instruments != nil { instruments.connections.Add(context.Background(), 1, metric.WithAttributeSet(attrs)) @@ -174,7 +176,7 @@ func RecordEventsReceivedBytes(ctx context.Context, instruments *Instruments, em return } ua, wrapper, route, method, appID, appVersion, instanceID := ri.sanitized() - attrs := buildRequestAttributes(em.envKVs, platformCategory, ua, wrapper, route, method, ri.URLScheme, appID, appVersion, instanceID) + attrs := buildRequestAttributes(em.attributes().kvs, platformCategory, ua, wrapper, route, method, ri.URLScheme, appID, appVersion, instanceID) instruments.eventsReceivedBytes.Add(ctx, bytes, metric.WithAttributeSet(attrs)) } @@ -185,7 +187,7 @@ func RecordRequestDuration(ctx context.Context, instruments *Instruments, em *En return } ua, wrapper, route, method, appID, appVersion, instanceID := ri.sanitized() - attrs := buildDurationAttributes(em.envKVs, measure.platformCategory, ua, wrapper, route, method, appID, appVersion, instanceID, ri.URLScheme, ri.ProtocolVersion, ri.ErrorType, ri.StatusCode) + attrs := buildDurationAttributes(em.attributes().kvs, measure.platformCategory, ua, wrapper, route, method, appID, appVersion, instanceID, ri.URLScheme, ri.ProtocolVersion, ri.ErrorType, ri.StatusCode) instruments.requestDuration.Record(ctx, duration.Seconds(), metric.WithAttributeSet(attrs)) } @@ -195,8 +197,7 @@ func RecordRequestDuration(ctx context.Context, instruments *Instruments, em *En // detached from any specific HTTP request context. type EventMetricsRecorder struct { instruments *Instruments - envKVs []attribute.KeyValue // private copy, safe for concurrent read - envAttrs attribute.Set // pre-computed to avoid concurrent sort in attribute.NewSet + env *EnvironmentManager // attributes are read per record, so a rename is picked up } // RecordDroppedEvents records the number of events dropped due to capacity overflow. @@ -204,7 +205,7 @@ func (r *EventMetricsRecorder) RecordDroppedEvents(count int) { if r.instruments == nil || count <= 0 { return } - r.instruments.eventsDropped.Add(context.Background(), int64(count), metric.WithAttributeSet(r.envAttrs)) + r.instruments.eventsDropped.Add(context.Background(), int64(count), metric.WithAttributeSet(r.env.attributes().set)) } // RecordEventsSent records the number of events successfully delivered to the events service. @@ -212,7 +213,7 @@ func (r *EventMetricsRecorder) RecordEventsSent(count int) { if r.instruments == nil || count <= 0 { return } - r.instruments.eventsSent.Add(context.Background(), int64(count), metric.WithAttributeSet(r.envAttrs)) + r.instruments.eventsSent.Add(context.Background(), int64(count), metric.WithAttributeSet(r.env.attributes().set)) } // RecordPendingEvents records the current number of events pending delivery. @@ -220,7 +221,7 @@ func (r *EventMetricsRecorder) RecordPendingEvents(depth int) { if r.instruments == nil { return } - r.instruments.pendingEvents.Record(context.Background(), int64(depth), metric.WithAttributeSet(r.envAttrs)) + r.instruments.pendingEvents.Record(context.Background(), int64(depth), metric.WithAttributeSet(r.env.attributes().set)) } // RecordEventsBytesSent records the size of event payloads successfully delivered. @@ -228,7 +229,7 @@ func (r *EventMetricsRecorder) RecordEventsBytesSent(bytes int) { if r.instruments == nil || bytes <= 0 { return } - r.instruments.eventsBytesSent.Add(context.Background(), int64(bytes), metric.WithAttributeSet(r.envAttrs)) + r.instruments.eventsBytesSent.Add(context.Background(), int64(bytes), metric.WithAttributeSet(r.env.attributes().set)) } // RecordEventsFailedSend records the number of events that could not be delivered after all retries. @@ -237,8 +238,9 @@ func (r *EventMetricsRecorder) RecordEventsFailedSend(count int, metadata ldeven if r.instruments == nil || count <= 0 { return } - kvs := make([]attribute.KeyValue, len(r.envKVs), len(r.envKVs)+1) - copy(kvs, r.envKVs) + envKVs := r.env.attributes().kvs + kvs := make([]attribute.KeyValue, len(envKVs), len(envKVs)+1) + copy(kvs, envKVs) kvs = append(kvs, statusCodeAttrKey.Int(metadata.StatusCode)) attrs := attribute.NewSet(kvs...) r.instruments.eventsFailedSend.Add(context.Background(), int64(count), metric.WithAttributeSet(attrs)) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 7109f6b5..74f653dd 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -5,6 +5,7 @@ import ( "errors" "log/slog" "sync" + "sync/atomic" "time" "github.com/launchdarkly/ld-relay/v9/config" @@ -51,13 +52,41 @@ type shutdown struct { closed chan struct{} } +// envAttributes is an immutable snapshot of an environment's metric attributes. Renaming an +// environment replaces the whole snapshot rather than mutating it, so readers need no +// synchronization beyond the atomic load that hands them one. +type envAttributes struct { + kvs []attribute.KeyValue + set attribute.Set +} + +func newEnvAttributes(relayID, envName, envID string) *envAttributes { + kvs := []attribute.KeyValue{ + relayIDAttrKey.String(relayID), + envNameAttrKey.String(tracing.SanitizeAttributeValue(envName)), + } + if envID != "" { + kvs = append(kvs, envIDAttrKey.String(envID)) + } + // NewSet sorts kvs in place; doing it here, before the snapshot is published, means every + // later reader gets an already-sorted slice it only ever copies from. + return &envAttributes{kvs: kvs, set: attribute.NewSet(kvs...)} +} + // EnvironmentManager controls the metrics exporter activity for a specific LD environment. type EnvironmentManager struct { - envKVs []attribute.KeyValue + relayID string + envID string + attrs atomic.Pointer[envAttributes] collector *RelayMetricsCollector closeOnce sync.Once } +// attributes returns the current attribute snapshot for this environment. +func (em *EnvironmentManager) attributes() *envAttributes { + return em.attrs.Load() +} + // NewManager creates a Manager instance. func NewManager( otlpConfig config.OpenTelemetryConfig, @@ -228,28 +257,25 @@ func (m *Manager) Close() { } // AddEnvironment creates a new EnvironmentManager with its own attribute set that includes -// the environment name. -func (m *Manager) AddEnvironment(envName string, publisher events.EventPublisher) (*EnvironmentManager, error) { +// the environment name, and the environment ID when one is available. +func (m *Manager) AddEnvironment(envName, envID string, publisher events.EventPublisher) (*EnvironmentManager, error) { m.lock.Lock() defer m.lock.Unlock() if m.closed { return nil, errAddEnvironmentAfterClosed } - envKVs := []attribute.KeyValue{ - relayIDAttrKey.String(m.metricsRelayID), - envNameAttrKey.String(sanitizeTagValue(envName)), - } - var collector *RelayMetricsCollector if publisher != nil { collector = newRelayMetricsCollector(m.metricsRelayID, envName, publisher, m.flushInterval, m.logger) } em := &EnvironmentManager{ - envKVs: envKVs, + relayID: m.metricsRelayID, + envID: envID, collector: collector, } + em.attrs.Store(newEnvAttributes(m.metricsRelayID, envName, envID)) m.environments = append(m.environments, em) return em, nil } @@ -282,24 +308,31 @@ func (m *Manager) RemoveEnvironmentForUsage(envName string) { m.usageChan <- removeEnvironment{envName: envName} } +// SetEnvironmentName rebuilds this environment's metric attributes around a new environment name. +// Relay calls this when an environment is renamed upstream, in automatic configuration or offline +// mode, so that metrics track the rename the same way spans do. Data points recorded afterwards +// carry the new name, which means the backend sees a new time series and the old one goes stale. +// +// The environment ID cannot change for a live environment, so it is carried over. +func (em *EnvironmentManager) SetEnvironmentName(envName string) { + em.attrs.Store(newEnvAttributes(em.relayID, envName, em.envID)) +} + // GetAttributes returns the attribute set for this EnvironmentManager. func (em *EnvironmentManager) GetAttributes() attribute.Set { - return attribute.NewSet(em.envKVs...) + return em.attributes().set } // NewEventMetricsRecorder creates an EventMetricsRecorder that records event processing metrics // with this environment's attributes. The returned recorder satisfies the EventMetrics interfaces // defined in both the events package and go-sdk-events. // -// The recorder makes a private copy of the environment attributes to avoid data races with -// attribute.NewSet's in-place sort. +// The recorder reads the environment's attributes when it records rather than capturing them here, +// so event metrics follow a rename like every other metric does. func (em *EnvironmentManager) NewEventMetricsRecorder(instruments *Instruments) *EventMetricsRecorder { - envKVsCopy := make([]attribute.KeyValue, len(em.envKVs)) - copy(envKVsCopy, em.envKVs) return &EventMetricsRecorder{ instruments: instruments, - envKVs: envKVsCopy, - envAttrs: attribute.NewSet(envKVsCopy...), + env: em, } } diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 261f9843..87674eba 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -41,11 +41,31 @@ func TestAddEnvironmentWithoutEventPublisher(t *testing.T) { require.NoError(t, err) defer manager.Close() - env, err := manager.AddEnvironment("name", nil) + env, err := manager.AddEnvironment("name", "", nil) assert.NoError(t, err) require.NotNil(t, env) assert.NotEqual(t, attribute.Set{}, env.GetAttributes()) + + // With no environment ID, the environment.id attribute is omitted. + attrs := env.GetAttributes() + _, ok := attrs.Value(envIDAttrKey) + assert.False(t, ok) +} + +func TestAddEnvironmentIncludesEnvironmentIDWhenProvided(t *testing.T) { + manager, err := NewManager(config.OpenTelemetryConfig{}, 0, slog.Default()) + require.NoError(t, err) + defer manager.Close() + + env, err := manager.AddEnvironment("name", "my-env-id", nil) + require.NoError(t, err) + require.NotNil(t, env) + + attrs := env.GetAttributes() + value, ok := attrs.Value(envIDAttrKey) + require.True(t, ok) + assert.Equal(t, "my-env-id", value.AsString()) } func TestAddEnvironmentWithEventPublisher(t *testing.T) { @@ -55,7 +75,7 @@ func TestAddEnvironmentWithEventPublisher(t *testing.T) { require.NoError(t, err) defer manager.Close() - env, err := manager.AddEnvironment("name", publisher) + env, err := manager.AddEnvironment("name", "", publisher) assert.NoError(t, err) require.NotNil(t, env) @@ -75,7 +95,7 @@ func TestAddEnvironmentAfterManagerClosed(t *testing.T) { manager, err := NewManager(config.OpenTelemetryConfig{}, 0, slog.Default()) require.NoError(t, err) manager.Close() - env, err := manager.AddEnvironment("name", nil) + env, err := manager.AddEnvironment("name", "", nil) assert.Nil(t, env) assert.Error(t, err) } @@ -85,7 +105,7 @@ func TestRemoveEnvironment(t *testing.T) { require.NoError(t, err) defer manager.Close() - env, err := manager.AddEnvironment("name", nil) + env, err := manager.AddEnvironment("name", "", nil) require.NoError(t, err) require.NotNil(t, env) @@ -342,7 +362,7 @@ func TestWithCountRecordsPolling(t *testing.T) { require.NoError(t, err) defer manager.Close() - env, err := manager.AddEnvironment("polling-test", publisher) + env, err := manager.AddEnvironment("polling-test", "", publisher) require.NoError(t, err) called := false @@ -378,13 +398,6 @@ func TestWithCountCallsFunctionForNonPollingMeasure(t *testing.T) { }) } -func TestSanitizeTagValue(t *testing.T) { - assert.Equal(t, "abc", sanitizeTagValue("abc")) - assert.Equal(t, "not-provided", sanitizeTagValue("")) - assert.Equal(t, "not-provided", sanitizeTagValue(" ")) - assert.Equal(t, "react_2.0.0", sanitizeTagValue("react/2.0.0")) -} - func TestSanitizeRouteValue(t *testing.T) { assert.Equal(t, "/sdk/evalx/contexts/{context}", sanitizeRouteValue("/sdk/evalx/contexts/{context}")) assert.Equal(t, "not-provided", sanitizeRouteValue("")) @@ -422,3 +435,85 @@ func assertGaugeValue(t *testing.T, m *metricdata.Metrics, envName, platform str // Ignore unused import warning - context is needed for p.collectMetrics var _ = context.Background + +func TestSetEnvironmentNameRebuildsAttributes(t *testing.T) { + manager, err := NewManager(config.OpenTelemetryConfig{}, 0, slog.Default()) + require.NoError(t, err) + defer manager.Close() + + env, err := manager.AddEnvironment("old name", "my-env-id", nil) + require.NoError(t, err) + + env.SetEnvironmentName("new/name") + + attrs := env.GetAttributes() + name, ok := attrs.Value(envNameAttrKey) + require.True(t, ok) + assert.Equal(t, "new_name", name.AsString(), "the new name is sanitized like the original") + + // The environment ID and relay ID are carried over. + envID, ok := attrs.Value(envIDAttrKey) + require.True(t, ok) + assert.Equal(t, "my-env-id", envID.AsString()) + relayID, ok := attrs.Value(relayIDAttrKey) + require.True(t, ok) + assert.Equal(t, manager.metricsRelayID, relayID.AsString()) +} + +func TestEnvironmentIDReachesExportedDataPoints(t *testing.T) { + testWithOTel(t, func(p testWithOTelParams) { + RecordEventsReceivedBytes(context.Background(), p.instruments, p.env, ServerPlatformCategory, + RequestInfo{UserAgent: userAgentValue, Route: "/bulk", Method: "POST"}, 1024) + + rm, err := p.collectMetrics() + require.NoError(t, err) + m := findMetric(rm, eventsReceivedMeasureName) + require.NotNil(t, m) + sum, ok := m.Data.(metricdata.Sum[int64]) + require.True(t, ok) + require.NotEmpty(t, sum.DataPoints) + + found := false + for _, dp := range sum.DataPoints { + envVal, envOK := dp.Attributes.Value(envNameAttrKey) + idVal, idOK := dp.Attributes.Value(envIDAttrKey) + if envOK && envVal.AsString() == p.envName { + require.True(t, idOK, "environment.id missing from the exported data point") + assert.Equal(t, p.envID, idVal.AsString()) + found = true + } + } + assert.True(t, found, "expected a data point for this environment") + }) +} + +func TestRenameChangesTheExportedEnvironmentName(t *testing.T) { + testWithOTel(t, func(p testWithOTelParams) { + recorder := p.env.NewEventMetricsRecorder(p.instruments) + recorder.RecordEventsSent(1) + + p.env.SetEnvironmentName("renamed env") + recorder.RecordEventsSent(1) + + rm, err := p.collectMetrics() + require.NoError(t, err) + m := findMetric(rm, eventsSentMeasureName) + require.NotNil(t, m) + sum, ok := m.Data.(metricdata.Sum[int64]) + require.True(t, ok) + + // The rename starts a new series rather than relabeling the old one, so both names are + // present -- and the event recorder followed the rename instead of pinning the old name. + names := make(map[string]int64) + for _, dp := range sum.DataPoints { + if envVal, envOK := dp.Attributes.Value(envNameAttrKey); envOK { + names[envVal.AsString()] = dp.Value + idVal, idOK := dp.Attributes.Value(envIDAttrKey) + require.True(t, idOK, "environment.id should survive a rename") + assert.Equal(t, p.envID, idVal.AsString()) + } + } + assert.Equal(t, int64(1), names[p.envName], "the pre-rename series keeps its data point") + assert.Equal(t, int64(1), names["renamed env"], "post-rename events land under the new name") + }) +} diff --git a/internal/metrics/test_utils_test.go b/internal/metrics/test_utils_test.go index 3c6df636..378e13bf 100644 --- a/internal/metrics/test_utils_test.go +++ b/internal/metrics/test_utils_test.go @@ -24,12 +24,14 @@ import ( const ( testMetricsRelayID = "test-metrics-relay-id" userAgentValue = "my-agent" + testEnvID = "test-env-id" ) type testWithOTelParams struct { manager *Manager relayID string envName string + envID string env *EnvironmentManager instruments *Instruments reader sdkmetric.Reader @@ -62,13 +64,14 @@ func testWithOTel(t *testing.T, action func(testWithOTelParams)) { // environment name for test isolation. envName := "env-" + uuid.New() - env, err := manager.AddEnvironment(envName, nil) + env, err := manager.AddEnvironment(envName, testEnvID, nil) require.NoError(t, err) action(testWithOTelParams{ manager: manager, relayID: manager.metricsRelayID, envName: envName, + envID: testEnvID, env: env, instruments: instruments, reader: reader, diff --git a/internal/middleware/auth_span_test.go b/internal/middleware/auth_span_test.go new file mode 100644 index 00000000..dd4de377 --- /dev/null +++ b/internal/middleware/auth_span_test.go @@ -0,0 +1,147 @@ +package middleware + +import ( + "net/http" + "testing" + + "github.com/launchdarkly/ld-relay/v9/internal/basictypes" + "github.com/launchdarkly/ld-relay/v9/internal/relayenv" + "github.com/launchdarkly/ld-relay/v9/internal/sdkauth" + st "github.com/launchdarkly/ld-relay/v9/internal/sharedtest" + "github.com/launchdarkly/ld-relay/v9/internal/sharedtest/testenv" + "github.com/launchdarkly/ld-relay/v9/internal/tracing" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testRequestSpanName stands in for the otelmux request span that wraps every real relay request. +const testRequestSpanName = "test.request" + +// withRecordedSpans installs a real (non-noop) tracer provider with an in-memory recorder for the +// duration of f, and returns the spans that were ended. +func withRecordedSpans(t *testing.T, f func()) tracetest.SpanStubs { + t.Helper() + recorder := tracetest.NewSpanRecorder() + tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(recorder)) + previous := otel.GetTracerProvider() + otel.SetTracerProvider(tp) + t.Cleanup(func() { otel.SetTracerProvider(previous) }) + + f() + + require.NoError(t, tp.ForceFlush(t.Context())) + return tracetest.SpanStubsFromReadOnlySpans(recorder.Ended()) +} + +// runWithRequestSpan sends req through selector inside a parent span, standing in for the otelmux +// request span, and returns every recorded span. +func runWithRequestSpan(t *testing.T, selector func(http.Handler) http.Handler, req *http.Request) tracetest.SpanStubs { + t.Helper() + return withRecordedSpans(t, func() { + ctx, requestSpan := tracing.Tracer().Start(req.Context(), testRequestSpanName) + defer requestSpan.End() + + resp, _ := st.DoRequest(req.WithContext(ctx), selector(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) +} + +func findSpan(t *testing.T, spans tracetest.SpanStubs, name string) tracetest.SpanStub { + t.Helper() + for _, s := range spans { + if s.Name == name { + return s + } + } + require.FailNow(t, "no span named "+name+" was recorded") + return tracetest.SpanStub{} +} + +func spanAttr(s tracetest.SpanStub, key attribute.Key) (attribute.Value, bool) { + for _, kv := range s.Attributes { + if kv.Key == key { + return kv.Value, true + } + } + return attribute.Value{}, false +} + +// The environment attributes go on the request span, not the auth span, so they cover every +// downstream handler span in the trace. +func TestEnvAttributesAreSetOnTheRequestSpan(t *testing.T) { + env := testenv.NewTestEnvContextWithEnvConfig("ProjectName JSClientSideEnv", st.EnvClientSide.Config, true, nil) + envs := testEnvironments{envs: map[sdkauth.ScopedCredential]relayenv.EnvContext{ + sdkauth.New(st.EnvClientSide.Config.SDKKey): env, + }} + selector := SelectEnvironmentByAuthorizationKey(basictypes.ServerSDK, envs) + + spans := runWithRequestSpan(t, selector, buildPreRoutedRequestWithAuth(st.EnvClientSide.Config.SDKKey)) + + requestSpan := findSpan(t, spans, testRequestSpanName) + name, ok := spanAttr(requestSpan, tracing.EnvNameKey) + require.True(t, ok, "environment.name missing from the request span") + assert.Equal(t, "ProjectName JSClientSideEnv", name.AsString()) + + id, ok := spanAttr(requestSpan, tracing.EnvIDKey) + require.True(t, ok, "environment.id missing from the request span") + assert.Equal(t, string(st.EnvClientSide.Config.EnvID), id.AsString()) + + // The auth span still reports the auth outcome, and does not duplicate the environment. + authSpan := findSpan(t, spans, tracing.SpanAuth) + result, ok := spanAttr(authSpan, tracing.AuthResultKey) + require.True(t, ok) + assert.Equal(t, "success", result.AsString()) + _, ok = spanAttr(authSpan, tracing.EnvNameKey) + assert.False(t, ok, "the environment belongs on the request span, not the auth span") +} + +// A filtered environment's display name contains a slash, which the sanitizer replaces so the span +// and the environment.name metric attribute report the same form of the name. +func TestEnvSpanNameIsSanitized(t *testing.T) { + env := testenv.NewTestEnvContext("ProjectName Production/mobile", true, nil) + envs := testEnvironments{envs: map[sdkauth.ScopedCredential]relayenv.EnvContext{ + sdkauth.New(st.EnvMain.Config.SDKKey): env, + }} + selector := SelectEnvironmentByAuthorizationKey(basictypes.ServerSDK, envs) + + spans := runWithRequestSpan(t, selector, buildPreRoutedRequestWithAuth(st.EnvMain.Config.SDKKey)) + + requestSpan := findSpan(t, spans, testRequestSpanName) + name, ok := spanAttr(requestSpan, tracing.EnvNameKey) + require.True(t, ok, "environment.name missing from the request span") + assert.Equal(t, "ProjectName Production_mobile", name.AsString()) + + // This environment has no client-side environment ID, so the ID attribute is omitted entirely. + _, ok = spanAttr(requestSpan, tracing.EnvIDKey) + assert.False(t, ok, "environment.id should be omitted when no EnvironmentID is configured") +} + +// The client-side auth middleware sets the same attributes on the same span. +func TestEnvAttributesAreSetOnTheRequestSpanForClientSideAuth(t *testing.T) { + env := testenv.NewTestEnvContextWithEnvConfig("ProjectName JSClientSideEnv", st.EnvClientSide.Config, true, nil) + envs := testEnvironments{envs: map[sdkauth.ScopedCredential]relayenv.EnvContext{ + sdkauth.New(st.EnvClientSide.Config.EnvID): env, + }} + selector := SelectEnvironmentByClientSideAuth(envs) + + // An environment ID carries no authorization header value of its own, so set it directly the + // way a browser SDK does. + headers := make(http.Header) + headers.Set("Authorization", string(st.EnvClientSide.Config.EnvID)) + spans := runWithRequestSpan(t, selector, buildPreRoutedRequest("GET", nil, headers, nil, nil)) + + requestSpan := findSpan(t, spans, testRequestSpanName) + name, ok := spanAttr(requestSpan, tracing.EnvNameKey) + require.True(t, ok, "environment.name missing from the request span") + assert.Equal(t, "ProjectName JSClientSideEnv", name.AsString()) + + id, ok := spanAttr(requestSpan, tracing.EnvIDKey) + require.True(t, ok, "environment.id missing from the request span") + assert.Equal(t, string(st.EnvClientSide.Config.EnvID), id.AsString()) +} diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 6fd9c63e..6d66fc90 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -1,6 +1,7 @@ package middleware import ( + "context" "encoding/base64" "encoding/json" "errors" @@ -24,7 +25,9 @@ import ( ld "github.com/launchdarkly/go-server-sdk/v7" "github.com/gorilla/mux" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" ) const ( @@ -98,6 +101,28 @@ func parseApplicationTags(req *http.Request) (applicationID, applicationVersion return } +// setEnvSpanAttributes records the authenticated environment on the span in ctx. Callers pass the +// request context rather than the auth-scoped one, so the attributes land on the request span -- +// the parent of both the auth span and every handler span below it -- making a whole trace +// filterable by environment. +// +// The keys are the same ones the metrics use, and the display name is sanitized the same way, so a +// trace and a metric series for one environment can be joined on either attribute. A rename keeps +// them in step: this reads the identifiers per request, and SetIdentifiers rebuilds the metric +// attributes to match. +// +// The environment ID is only set when an EnvironmentID credential is configured; it is absent for +// SDK-key-only environments in a manual configuration. +func setEnvSpanAttributes(ctx context.Context, clientCtx relayenv.EnvContext) { + attrs := []attribute.KeyValue{ + tracing.EnvNameKey.String(tracing.SanitizeAttributeValue(clientCtx.GetIdentifiers().GetDisplayName())), + } + if envID := relayenv.GetEnvironmentID(clientCtx); envID != "" { + attrs = append(attrs, tracing.EnvIDKey.String(string(envID))) + } + trace.SpanFromContext(ctx).SetAttributes(attrs...) +} + // Chain combines a series of middleware functions that will be applied in the same order. func Chain(middlewares ...mux.MiddlewareFunc) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { @@ -186,6 +211,9 @@ func SelectEnvironmentByAuthorizationKey(sdkKind basictypes.SDKKind, envs RelayE span.SetAttributes(tracing.AuthResultKey.String("success")) + // req still carries the pre-auth context here, so this targets the request span. + setEnvSpanAttributes(req.Context(), clientCtx) + contextInfo := EnvContextInfo{ Env: clientCtx, Credential: credential, @@ -294,6 +322,9 @@ func SelectEnvironmentByClientSideAuth(envs RelayEnvironments) mux.MiddlewareFun span.SetAttributes(tracing.AuthResultKey.String("success")) + // req still carries the pre-auth context here, so this targets the request span. + setEnvSpanAttributes(req.Context(), clientCtx) + contextInfo := EnvContextInfo{ Env: clientCtx, Credential: cred, diff --git a/internal/relayenv/env_context_impl.go b/internal/relayenv/env_context_impl.go index 88fb6b53..3aa05b66 100644 --- a/internal/relayenv/env_context_impl.go +++ b/internal/relayenv/env_context_impl.go @@ -303,7 +303,7 @@ func NewEnvContext( envContext.metricsEventPub = eventsPublisher } - em, err = params.MetricsManager.AddEnvironment(params.Identifiers.GetDisplayName(), envContext.metricsEventPub) + em, err = params.MetricsManager.AddEnvironment(params.Identifiers.GetDisplayName(), string(envConfig.EnvID), envContext.metricsEventPub) if err != nil { return nil, errInitMetrics(err) } @@ -577,6 +577,12 @@ func (c *envContextImpl) SetIdentifiers(ei EnvIdentifiers) { defer c.mu.Unlock() c.identifiers = ei + + // Keep the metric attributes in step with the display name the spans report. This starts a new + // metric time series under the new name; the old one stops receiving data points. + if c.metricsEnv != nil { + c.metricsEnv.SetEnvironmentName(ei.GetDisplayName()) + } } func (c *envContextImpl) UpdateCredential(update *CredentialUpdate) { diff --git a/internal/relayenv/env_context_impl_test.go b/internal/relayenv/env_context_impl_test.go index 66f6ac1f..bcd940b1 100644 --- a/internal/relayenv/env_context_impl_test.go +++ b/internal/relayenv/env_context_impl_test.go @@ -37,6 +37,8 @@ import ( helpers "github.com/launchdarkly/go-test-helpers/v3" "github.com/launchdarkly/go-test-helpers/v3/httphelpers" + "go.opentelemetry.io/otel/attribute" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -754,3 +756,38 @@ func ensureSynchronizerState(t *testing.T, synchronizer *mockBigSegmentSynchroni return synchronizer.isStarted() == expectedState }, time.Second, 10*time.Millisecond, "timed out waiting for big segments synchronizer to start") } + +func TestSetIdentifiersUpdatesTheMetricsEnvironmentName(t *testing.T) { + metricsManager, err := metrics.NewManager(config.OpenTelemetryConfig{}, time.Minute, slog.Default()) + require.NoError(t, err) + defer metricsManager.Close() + + env, err := NewEnvContext(EnvContextImplParams{ + Identifiers: EnvIdentifiers{ProjName: "My Proj", EnvName: "Production"}, + EnvConfig: st.EnvClientSide.Config, + ClientFactory: testclient.FakeLDClientFactory(true), + MetricsManager: metricsManager, + Logger: slog.Default(), + }, nil) + require.NoError(t, err) + defer env.Close() + + nameAttr := func() string { + attrs := env.GetMetricsEnv().GetAttributes() + value, ok := attrs.Value(attribute.Key("environment.name")) + require.True(t, ok) + return value.AsString() + } + require.Equal(t, "My Proj Production", nameAttr()) + + // An upstream rename arrives, the way auto-configuration and offline mode deliver one. + env.SetIdentifiers(EnvIdentifiers{ProjName: "My Proj", EnvName: "Prod"}) + + assert.Equal(t, "My Proj Prod", env.GetIdentifiers().GetDisplayName()) + assert.Equal(t, "My Proj Prod", nameAttr(), "the metric attribute follows the rename") + + attrs := env.GetMetricsEnv().GetAttributes() + envIDAttr, ok := attrs.Value(attribute.Key("environment.id")) + require.True(t, ok) + assert.Equal(t, string(st.EnvClientSide.Config.EnvID), envIDAttr.AsString(), "the environment ID is preserved") +} diff --git a/internal/tracing/attributes.go b/internal/tracing/attributes.go index ec864c7e..1ac4efed 100644 --- a/internal/tracing/attributes.go +++ b/internal/tracing/attributes.go @@ -1,6 +1,8 @@ package tracing import ( + "strings" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -36,3 +38,26 @@ const ( PayloadBytesKey = attribute.Key("relay.payload.bytes") ResponseBytesKey = attribute.Key("relay.response.bytes") ) + +// Attribute keys identifying a LaunchDarkly environment. Spans and metrics use the same keys so +// that a trace and a metric series for one environment can be joined on either of them. +const ( + EnvNameKey = attribute.Key("environment.name") + EnvIDKey = attribute.Key("environment.id") +) + +// SanitizeAttributeValue ensures telemetry attribute values are valid. Blank values are replaced +// with a descriptive default, surrounding whitespace is trimmed, and slashes are replaced with +// underscores. This is appropriate for free-form values such as environment names, user agent +// strings, and SDK wrapper names, but not for routes, where slashes are meaningful. +// +// A value that reaches both spans and metrics must go through this in both places, so that each +// signal reports it in the same form. Distinct values can collapse onto the same sanitized value +// (both "a/b" and "a_b" become "a_b"), which merges them wherever they are used as an attribute. +func SanitizeAttributeValue(v string) string { + trimmed := strings.TrimSpace(v) + if trimmed == "" { + return "not-provided" + } + return strings.ReplaceAll(trimmed, "/", "_") +} diff --git a/internal/tracing/attributes_test.go b/internal/tracing/attributes_test.go new file mode 100644 index 00000000..18df38c9 --- /dev/null +++ b/internal/tracing/attributes_test.go @@ -0,0 +1,20 @@ +package tracing + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSanitizeAttributeValue(t *testing.T) { + assert.Equal(t, "abc", SanitizeAttributeValue("abc")) + assert.Equal(t, "not-provided", SanitizeAttributeValue("")) + assert.Equal(t, "not-provided", SanitizeAttributeValue(" ")) + assert.Equal(t, "react_2.0.0", SanitizeAttributeValue("react/2.0.0")) + assert.Equal(t, "My Project_My Env", SanitizeAttributeValue("My Project/My Env")) + + // An auto-configured environment with no project name computes " Production" as its display + // name; without trimming it would be a separate series from "Production" but render identically. + assert.Equal(t, "Production", SanitizeAttributeValue(" Production")) + assert.Equal(t, "Production", SanitizeAttributeValue("Production\t")) +} diff --git a/relay/relay_endpoints_spans_test.go b/relay/relay_endpoints_spans_test.go index 0fbadea7..45fc6c8c 100644 --- a/relay/relay_endpoints_spans_test.go +++ b/relay/relay_endpoints_spans_test.go @@ -278,3 +278,53 @@ func TestPollingEndpointSpansDoNotLeakOnEarlyReturn(t *testing.T) { assert.Zero(t, countStarted(started, tracing.SpanWriteResponse)) }) } + +// TestRequestSpanCarriesEnvironmentAttributes drives requests through the full relay so the real +// otelmux request span is the root, and asserts the auth middlewares put the authenticated +// environment on that span -- where it covers every handler span in the trace -- using the same +// attribute keys the metrics use. +func TestRequestSpanCarriesEnvironmentAttributes(t *testing.T) { + recorder := installSpanRecorder(t) + + var config c.Config + config.Environment = st.MakeEnvConfigs(st.EnvMain, st.EnvClientSide) + + withStartedRelay(t, config, func(p relayTestParams) { + t.Run("server-side SDK key", func(t *testing.T) { + recorder.Reset() + + result, _ := st.DoRequest(st.BuildRequestWithAuth("GET", "/sdk/flags", st.EnvMain.Config.SDKKey, nil), p.relay) + require.Equal(t, http.StatusOK, result.StatusCode) + + attrs := spanAttrs(rootSpan(t, recorder.Ended())) + name, ok := attrs[tracing.EnvNameKey] + require.True(t, ok, "request span is missing environment.name") + assert.Equal(t, string(st.EnvMain.Name), name.AsString()) + + // EnvMain is configured with an SDK key only, so there is no environment ID to report. + _, ok = attrs[tracing.EnvIDKey] + assert.False(t, ok, "environment.id should be absent for an SDK-key-only environment") + }) + + t.Run("client-side environment ID", func(t *testing.T) { + recorder.Reset() + + // The client-side auth middleware takes the environment ID in the Authorization + // header, and serves the unified mobile/JS client polling endpoint. + headers := make(http.Header) + headers.Set("Authorization", string(st.EnvClientSide.Config.EnvID)) + contextParam := base64.StdEncoding.EncodeToString([]byte(`{"kind":"user","key":"me"}`)) + result, _ := st.DoRequest(st.BuildRequest("GET", "/sdk/poll/eval/"+contextParam, nil, headers), p.relay) + require.Equal(t, http.StatusOK, result.StatusCode) + + attrs := spanAttrs(rootSpan(t, recorder.Ended())) + name, ok := attrs[tracing.EnvNameKey] + require.True(t, ok, "request span is missing environment.name") + assert.Equal(t, string(st.EnvClientSide.Name), name.AsString()) + + envID, ok := attrs[tracing.EnvIDKey] + require.True(t, ok, "request span is missing environment.id") + assert.Equal(t, string(st.EnvClientSide.Config.EnvID), envID.AsString()) + }) + }) +}