feat: add support for separate metrics tab independent of traces for OTEL

helm-charts/bifrost/values.schema.json

@@ -1059,11 +1059,12 @@
                   "type": "object",
                   "properties": {
                     "service_name": {
-                      "type": "string"
+                      "type": "string",
+                      "description": "Name of the service to report to Datadog. Supports env.VAR_NAME prefix for environment variable substitution (e.g. env.BIFROST_DD_SERVICE)"
                     },
                     "ml_app": {
                       "type": "string",
-                      "description": "ML application name for Datadog LLM Observability grouping (defaults to service_name)"
+                      "description": "ML application name for Datadog LLM Observability grouping (defaults to service_name). Supports env.VAR_NAME prefix for environment variable substitution (e.g. env.BIFROST_DD_ML_APP)"
                     },
                     "agent_addr": {
                       "type": "string",
@@ -1090,10 +1091,12 @@
                       "description": "DogStatsD server port for metrics, used with dogstatsd_host (agent mode only). Supports env.VAR_NAME prefix. Defaults to 8125; has no effect when dogstatsd_host is unset"
                     },
                     "env": {
-                      "type": "string"
+                      "type": "string",
+                      "description": "Environment tag (e.g. production, staging). Supports env.VAR_NAME prefix for environment variable substitution (e.g. env.BIFROST_DD_ENV)"
                     },
                     "version": {
-                      "type": "string"
+                      "type": "string",
+                      "description": "Service version tag. Supports env.VAR_NAME prefix for environment variable substitution (e.g. env.BIFROST_DD_VERSION)"
                     },
                     "custom_tags": {
                       "type": "object",

helm-charts/bifrost/values.yaml

@@ -658,7 +658,7 @@ bifrost:
       enabled: false
       version: 1
       config:
-        service_name: "bifrost"
+        service_name: "bifrost"  # Service name reported to Datadog (supports env.VAR_NAME)
         # Datadog Agent address. Supports env.VAR_NAME references — e.g. set
         # agent_addr: "env.DD_AGENT_ADDR" and inject DD_AGENT_ADDR via the
         # top-level `env:` (e.g. from status.hostIP for a node-local agent DaemonSet).
@@ -672,11 +672,11 @@ bifrost:
         # agent_port: "8126"
         # dogstatsd_host: "env.DD_AGENT_HOST"
         # dogstatsd_port: "8125"
-        env: ""
-        version: ""
+        env: ""                     # Environment tag, e.g. production (supports env.VAR_NAME)
+        version: ""                 # Service version tag (supports env.VAR_NAME)
         custom_tags: {}
         enable_traces: true
-        # ml_app: ""                  # ML app name for LLM Observability (defaults to service_name)
+        # ml_app: ""                  # ML app name for LLM Observability (defaults to service_name, supports env.VAR_NAME)
         # enable_metrics: true
         # enable_llm_obs: true
         # group_traces_by_session: false  # Group requests sharing x-bf-session-id into one APM trace (agent mode only)

plugins/otel/main.go

@@ -87,6 +87,11 @@ type Profile struct {
 	// a disabled profile builds no trace client or metrics exporter, so no traces/metrics
 	// are sent for it. Defaults to true when omitted.
 	Enabled      bool               `json:"enabled"`
+
+	// TracesEnabled gates trace export. When false, no trace client is built and
+	// CollectorURL is not required: a metrics-only profile. Defaults to true when omitted.
+	TracesEnabled bool `json:"traces_enabled"`
+
 	ServiceName  string             `json:"service_name"`
 	CollectorURL *schemas.SecretVar `json:"collector_url"`
 	Headers      map[string]string  `json:"headers,omitempty"`
@@ -137,8 +142,9 @@ type Profile struct {
 func (p *Profile) UnmarshalJSON(data []byte) error {
 	type alias Profile
 	aux := struct {
-		Enabled  *bool `json:"enabled"`
-		Insecure *bool `json:"insecure"`
+		Enabled       *bool `json:"enabled"`
+		Insecure      *bool `json:"insecure"`
+		TracesEnabled *bool `json:"traces_enabled"`
 		*alias
 	}{
 		alias: (*alias)(p),
@@ -156,6 +162,12 @@ func (p *Profile) UnmarshalJSON(data []byte) error {
 	} else {
 		p.Enabled = *aux.Enabled
 	}
+	// Default traces on so existing configs keep exporting spans.
+	if aux.TracesEnabled == nil {
+		p.TracesEnabled = true
+	} else {
+		p.TracesEnabled = *aux.TracesEnabled
+	}
 	return nil
 }
 
@@ -238,6 +250,7 @@ func hoistSpanFilter(data []byte) *PluginSpanFilter {
 // persistence.
 type profileForStorage struct {
 	Enabled                bool              `json:"enabled"`
+	TracesEnabled          bool              `json:"traces_enabled"`
 	ServiceName            string            `json:"service_name"`
 	CollectorURL           string            `json:"collector_url"`
 	Headers                map[string]string `json:"headers,omitempty"`
@@ -276,6 +289,7 @@ func (c *Config) MarshalForStorage() ([]byte, error) {
 		}
 		out.Profiles = append(out.Profiles, profileForStorage{
 			Enabled:                p.Enabled,
+			TracesEnabled:          p.TracesEnabled,
 			ServiceName:            p.ServiceName,
 			CollectorURL:           schemas.SecretVarAsString(p.CollectorURL),
 			Headers:                p.Headers,
@@ -509,15 +523,22 @@ func (p *OtelPlugin) buildTarget(index int, profile *Profile) (*otelTarget, erro
 	if profile == nil {
 		return nil, fmt.Errorf("profile %d is nil", index)
 	}
-	if profile.CollectorURL == nil || profile.CollectorURL.GetValue() == "" {
-		return nil, fmt.Errorf("profile %d: collector url is required", index)
-	}
 
 	serviceName := profile.ServiceName
 	if serviceName == "" {
 		serviceName = "bifrost"
 	}
 
+	// Both traces and metrics dial with this protocol, so validate it once. A profile
+	// with neither enabled is a no-op, so skip the check.
+	if profile.TracesEnabled || profile.MetricsEnabled {
+		switch profile.Protocol {
+		case ProtocolGRPC, ProtocolHTTP:
+		default:
+			return nil, fmt.Errorf("profile %d: invalid protocol type %q", index, profile.Protocol)
+		}
+	}
+
 	// Copy headers before resolving so the stored config is never mutated, then resolve
 	// any "env." references against the environment (errors if a referenced var is unset).
 	headers := make(map[string]string, len(profile.Headers))
@@ -531,10 +552,8 @@ func (p *OtelPlugin) buildTarget(index int, profile *Profile) (*otelTarget, erro
 		return nil, fmt.Errorf("profile %d: %w", index, err)
 	}
 
-	url := profile.CollectorURL.GetValue()
 	target := &otelTarget{
 		serviceName:            serviceName,
-		url:                    url,
 		traceType:              profile.TraceType,
 		requestHeaders:         slices.Clone(profile.RequestHeaders),
 		disableContentLogging:  profile.DisableContentLogging,
@@ -543,31 +562,41 @@ func (p *OtelPlugin) buildTarget(index int, profile *Profile) (*otelTarget, erro
 		exportTimeout:          exportTimeout,
 	}
 
-	switch profile.Protocol {
-	case ProtocolGRPC:
-		// gRPC has no client-side timeout of its own; the per-export context deadline
-		// applied in Inject is what bounds it.
-		target.client, err = NewOtelClientGRPC(url, headers, profile.TLSCACert, profile.Insecure)
-	case ProtocolHTTP:
-		target.client, err = NewOtelClientHTTP(url, headers, profile.TLSCACert, profile.Insecure, exportTimeout)
-	default:
-		return nil, fmt.Errorf("profile %d: invalid protocol type %q", index, profile.Protocol)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("profile %d: %w", index, err)
+	// Build the trace client only when traces are enabled; Inject skips a nil client.
+	if profile.TracesEnabled {
+		if profile.CollectorURL == nil || profile.CollectorURL.GetValue() == "" {
+			return nil, fmt.Errorf("profile %d: collector url is required when traces_enabled is true", index)
+		}
+		url := profile.CollectorURL.GetValue()
+		target.url = url
+		switch profile.Protocol {
+		case ProtocolGRPC:
+			// gRPC has no client-side timeout of its own; the per-export context deadline
+			// applied in Inject is what bounds it.
+			target.client, err = NewOtelClientGRPC(url, headers, profile.TLSCACert, profile.Insecure)
+		case ProtocolHTTP:
+			target.client, err = NewOtelClientHTTP(url, headers, profile.TLSCACert, profile.Insecure, exportTimeout)
+		}
+		if err != nil {
+			return nil, fmt.Errorf("profile %d: %w", index, err)
+		}
 	}
 
 	// Initialize metrics exporter if enabled
 	if profile.MetricsEnabled {
 		if profile.MetricsEndpoint.GetValue() == "" {
-			target.client.Close()
+			if target.client != nil {
+				target.client.Close()
+			}
 			return nil, fmt.Errorf("profile %d: metrics_endpoint is required when metrics_enabled is true", index)
 		}
 		pushInterval := profile.MetricsPushInterval
 		if pushInterval <= 0 {
 			pushInterval = 15 // default 15 seconds
 		} else if pushInterval > 300 {
-			target.client.Close()
+			if target.client != nil {
+				target.client.Close()
+			}
 			return nil, fmt.Errorf("profile %d: metrics_push_interval must be between 1 and 300 seconds, got %d", index, pushInterval)
 		}
 		metricsConfig := &MetricsConfig{

plugins/otel/profiles_test.go

@@ -355,6 +355,139 @@ func TestInitMultiProfileValidation(t *testing.T) {
 	}
 }
 
+// TestProfileTracesEnabledDefault verifies TracesEnabled defaults to true when omitted
+// and is honored when set explicitly.
+func TestProfileTracesEnabledDefault(t *testing.T) {
+	raw := `{
+		"profiles": [
+			{"collector_url": "a:4317", "trace_type": "genai_extension", "protocol": "grpc"},
+			{"protocol": "http", "metrics_enabled": true, "metrics_endpoint": "b:4318", "traces_enabled": false}
+		]
+	}`
+
+	var cfg Config
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if !cfg.Profiles[0].TracesEnabled {
+		t.Errorf("profile 0 TracesEnabled = false, want true (default)")
+	}
+	if cfg.Profiles[1].TracesEnabled {
+		t.Errorf("profile 1 TracesEnabled = true, want false (explicit)")
+	}
+}
+
+// TestInitMetricsOnlyProfile verifies a metrics-only profile (traces disabled, no
+// collector_url) builds a target with a metrics exporter but no trace client.
+func TestInitMetricsOnlyProfile(t *testing.T) {
+	raw := `{"profiles": [
+		{"traces_enabled": false, "protocol": "http", "metrics_enabled": true, "metrics_endpoint": "localhost:4318"}
+	]}`
+
+	var cfg Config
+	if err := sonic.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	plugin, err := Init(context.Background(), &cfg, testLogger{}, nil, "")
+	if err != nil {
+		t.Fatalf("Init metrics-only profile: %v", err)
+	}
+	t.Cleanup(func() { _ = plugin.Cleanup() })
+	if len(plugin.targets) != 1 {
+		t.Fatalf("targets len = %d, want 1", len(plugin.targets))
+	}
+	if plugin.targets[0].client != nil {
+		t.Errorf("metrics-only target has a trace client, want nil")
+	}
+	if plugin.targets[0].metricsExporter == nil {
+		t.Errorf("metrics-only target has no metrics exporter, want one")
+	}
+}
+
+// TestInitMetricsOnlyPushIntervalTooLarge verifies a metrics-only profile (nil trace
+// client) with metrics_push_interval > 300 returns the validation error instead of
+// panicking on a nil client.Close().
+func TestInitMetricsOnlyPushIntervalTooLarge(t *testing.T) {
+	raw := `{"profiles": [
+		{"traces_enabled": false, "protocol": "http", "metrics_enabled": true, "metrics_endpoint": "localhost:4318", "metrics_push_interval": 301}
+	]}`
+
+	var cfg Config
+	if err := sonic.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	plugin, err := Init(context.Background(), &cfg, testLogger{}, nil, "")
+	if err == nil {
+		if plugin != nil {
+			_ = plugin.Cleanup()
+		}
+		t.Fatalf("expected error for metrics_push_interval > 300, got nil")
+	}
+}
+
+// TestInitTracesOnlyProfileNeedsCollectorURL verifies a traces-enabled profile still
+// requires collector_url.
+func TestInitTracesOnlyProfileNeedsCollectorURL(t *testing.T) {
+	raw := `{"profiles": [
+		{"trace_type": "genai_extension", "protocol": "grpc"}
+	]}`
+
+	var cfg Config
+	if err := sonic.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if _, err := Init(context.Background(), &cfg, testLogger{}, nil, ""); err == nil {
+		t.Errorf("expected error for traces-enabled profile missing collector_url")
+	}
+}
+
+// TestInitBothDisabledProfile verifies a profile with both traces and metrics disabled
+// is allowed as a no-op (matching the telemetry plugin, where pull and push are
+// independent and both may be off): it builds a target with no client or exporter.
+func TestInitBothDisabledProfile(t *testing.T) {
+	raw := `{"profiles": [
+		{"traces_enabled": false}
+	]}`
+
+	var cfg Config
+	if err := sonic.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	plugin, err := Init(context.Background(), &cfg, testLogger{}, nil, "")
+	if err != nil {
+		t.Fatalf("Init both-disabled profile: %v", err)
+	}
+	t.Cleanup(func() { _ = plugin.Cleanup() })
+	if len(plugin.targets) != 1 {
+		t.Fatalf("targets len = %d, want 1", len(plugin.targets))
+	}
+	if plugin.targets[0].client != nil || plugin.targets[0].metricsExporter != nil {
+		t.Errorf("both-disabled target should have no client or exporter")
+	}
+}
+
+// TestTracesEnabledStorageRoundTrip verifies traces_enabled survives storage marshalling.
+func TestTracesEnabledStorageRoundTrip(t *testing.T) {
+	raw := `{"profiles": [
+		{"traces_enabled": false, "protocol": "http", "metrics_enabled": true, "metrics_endpoint": "localhost:4318"}
+	]}`
+	var cfg Config
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	stored, err := cfg.MarshalForStorage()
+	if err != nil {
+		t.Fatalf("MarshalForStorage: %v", err)
+	}
+	var back Config
+	if err := json.Unmarshal(stored, &back); err != nil {
+		t.Fatalf("round-trip unmarshal: %v", err)
+	}
+	if back.Profiles[0].TracesEnabled {
+		t.Errorf("round-trip TracesEnabled = true, want false")
+	}
+}
+
 type testLogger struct{}
 
 func (testLogger) Debug(string, ...any)                   {}

tests/e2e/features/observability/pages/observability.page.ts

@@ -179,6 +179,11 @@ export class ObservabilityPage extends BasePage {
    */
   async enableMetricsExport(): Promise<void> {
     await this.selectConnector('otel')
+    // The metrics-export toggle lives in the profile's Metrics tab, which is not the
+    // default active tab, so select it before interacting with the toggle.
+    const metricsTab = this.page.getByTestId('otel-profile-0-tab-metrics')
+    await metricsTab.waitFor({ state: 'visible', timeout: 5000 })
+    await metricsTab.click()
     const switch_ = this.page.getByTestId('otel-metrics-export-toggle')
     await switch_.waitFor({ state: 'visible', timeout: 5000 })
     const checked = await switch_.getAttribute('data-state') === 'checked'
@@ -290,6 +295,12 @@ export class ObservabilityPage extends BasePage {
    * so we also treat the "Enable Metrics Export" section as OTel content.
    */
   async isMetricsEndpointVisible(): Promise<boolean> {
+    // The metrics subsection lives in the profile's Metrics tab, which is not active by
+    // default; select it first so its content is mounted before checking visibility.
+    const metricsTab = this.page.getByTestId('otel-profile-0-tab-metrics')
+    await metricsTab.waitFor({ state: 'visible', timeout: 5000 }).catch(() => {})
+    await metricsTab.click().catch(() => {})
+
     // Metrics endpoint input (only visible when Enable Metrics Export is on)
     const metricsInputByValue = this.page.locator('input[value*="/metrics"]')
     const valueVisible = await metricsInputByValue.isVisible().catch(() => false)