Skip to content

Commit 6cf23d4

Browse files
jerm-drojerm-dro
authored
Refactor CRD Telemetry Config Conversion for Reusability (#2908)
Refactor CRD Telmetry Config Conversion for Reusability - Moved the telemetry.Config construction logic that was within WithTelemetryConfig to telemetry.MaybeMakeConfig. - Renamed WithTelemetryConfig to WithTelemetryConfigFlags which behaves just as before. - Created WithTelemetryConfig which just assign the given telemetry config. - Created spectoconfig/telemetry.go (pronounced "Spec To Config" other names welcome) which contains ConvertTelemetryConfig. ConvertTelemetryConfig is exactly the same as AddTelemetryConfigOptions minus creating the option. Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
1 parent 55d0cee commit 6cf23d4

7 files changed

Lines changed: 182 additions & 129 deletions

File tree

‎cmd/thv-operator/pkg/runconfig/telemetry.go‎

Lines changed: 4 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@ package runconfig
33

44
import (
55
"context"
6-
"strconv"
7-
"strings"
8-
9-
"sigs.k8s.io/controller-runtime/pkg/log"
106

117
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
8+
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/spectoconfig"
129
"github.com/stacklok/toolhive/pkg/runner"
1310
)
1411

@@ -19,79 +16,12 @@ func AddTelemetryConfigOptions(
1916
telemetryConfig *mcpv1alpha1.TelemetryConfig,
2017
mcpServerName string,
2118
) {
22-
if telemetryConfig == nil {
19+
if telemetryConfig == nil || options == nil {
2320
return
2421
}
2522

26-
// Default values
27-
var otelEndpoint string
28-
var otelEnablePrometheusMetricsPath bool
29-
var otelTracingEnabled bool
30-
var otelMetricsEnabled bool
31-
var otelServiceName string
32-
var otelSamplingRate = 0.05 // Default sampling rate
33-
var otelHeaders []string
34-
var otelInsecure bool
35-
var otelEnvironmentVariables []string
36-
37-
// Process OpenTelemetry configuration
38-
if telemetryConfig.OpenTelemetry != nil && telemetryConfig.OpenTelemetry.Enabled {
39-
otel := telemetryConfig.OpenTelemetry
40-
41-
// Strip http:// or https:// prefix if present, as OTLP client expects host:port format
42-
otelEndpoint = strings.TrimPrefix(strings.TrimPrefix(otel.Endpoint, "https://"), "http://")
43-
otelInsecure = otel.Insecure
44-
otelHeaders = otel.Headers
45-
46-
// Use MCPServer name as service name if not specified
47-
if otel.ServiceName != "" {
48-
otelServiceName = otel.ServiceName
49-
} else {
50-
otelServiceName = mcpServerName
51-
}
52-
53-
// Handle tracing configuration
54-
if otel.Tracing != nil {
55-
otelTracingEnabled = otel.Tracing.Enabled
56-
if otel.Tracing.SamplingRate != "" {
57-
// Parse sampling rate string to float64
58-
if rate, err := strconv.ParseFloat(otel.Tracing.SamplingRate, 64); err == nil {
59-
otelSamplingRate = rate
60-
} else {
61-
logger := log.FromContext(ctx)
62-
logger.Error(err, "Failed to parse sampling rate, using default",
63-
"samplingRate", otel.Tracing.SamplingRate,
64-
"default", otelSamplingRate,
65-
"mcpServer", mcpServerName)
66-
}
67-
}
68-
}
69-
70-
// Handle metrics configuration
71-
if otel.Metrics != nil {
72-
otelMetricsEnabled = otel.Metrics.Enabled
73-
}
74-
}
75-
76-
// Process Prometheus configuration
77-
if telemetryConfig.Prometheus != nil {
78-
otelEnablePrometheusMetricsPath = telemetryConfig.Prometheus.Enabled
79-
}
80-
81-
if options == nil {
82-
return
83-
}
23+
config := spectoconfig.ConvertTelemetryConfig(ctx, telemetryConfig, mcpServerName)
8424

8525
// Add telemetry config to options
86-
*options = append(*options, runner.WithTelemetryConfig(
87-
otelEndpoint,
88-
otelEnablePrometheusMetricsPath,
89-
otelTracingEnabled,
90-
otelMetricsEnabled,
91-
otelServiceName,
92-
otelSamplingRate,
93-
otelHeaders,
94-
otelInsecure,
95-
otelEnvironmentVariables,
96-
))
26+
*options = append(*options, runner.WithTelemetryConfig(config))
9727
}

‎cmd/thv-operator/pkg/spectoconfig/telemetry.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Package spectoconfig provides functionality to convert CRD Telemetry types into telemetry.Config.
2+
package spectoconfig
3+
4+
import (
5+
"context"
6+
"strconv"
7+
"strings"
8+
9+
"sigs.k8s.io/controller-runtime/pkg/log"
10+
11+
"github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
12+
"github.com/stacklok/toolhive/pkg/telemetry"
13+
)
14+
15+
// ConvertTelemetryConfig converts the CRD TelemetryConfig to a telemetry.Config.
16+
// It may return nil if no telemetry is configured.
17+
func ConvertTelemetryConfig(
18+
ctx context.Context,
19+
telemetryConfig *v1alpha1.TelemetryConfig,
20+
mcpServerName string,
21+
) *telemetry.Config {
22+
if telemetryConfig == nil {
23+
return nil
24+
}
25+
26+
// Default values
27+
var otelEndpoint string
28+
var otelEnablePrometheusMetricsPath bool
29+
var otelTracingEnabled bool
30+
var otelMetricsEnabled bool
31+
var otelServiceName string
32+
var otelSamplingRate = 0.05 // Default sampling rate
33+
var otelHeaders []string
34+
var otelInsecure bool
35+
var otelEnvironmentVariables []string
36+
37+
// Process OpenTelemetry configuration
38+
if telemetryConfig.OpenTelemetry != nil && telemetryConfig.OpenTelemetry.Enabled {
39+
otel := telemetryConfig.OpenTelemetry
40+
41+
// Strip http:// or https:// prefix if present, as OTLP client expects host:port format
42+
otelEndpoint = strings.TrimPrefix(strings.TrimPrefix(otel.Endpoint, "https://"), "http://")
43+
otelInsecure = otel.Insecure
44+
otelHeaders = otel.Headers
45+
46+
// Use MCPServer name as service name if not specified
47+
if otel.ServiceName != "" {
48+
otelServiceName = otel.ServiceName
49+
} else {
50+
otelServiceName = mcpServerName
51+
}
52+
53+
// Handle tracing configuration
54+
if otel.Tracing != nil {
55+
otelTracingEnabled = otel.Tracing.Enabled
56+
if otel.Tracing.SamplingRate != "" {
57+
// Parse sampling rate string to float64
58+
if rate, err := strconv.ParseFloat(otel.Tracing.SamplingRate, 64); err == nil {
59+
otelSamplingRate = rate
60+
} else {
61+
logger := log.FromContext(ctx)
62+
logger.Error(err, "Failed to parse sampling rate, using default",
63+
"samplingRate", otel.Tracing.SamplingRate,
64+
"default", otelSamplingRate,
65+
"mcpServer", mcpServerName)
66+
}
67+
}
68+
}
69+
70+
// Handle metrics configuration
71+
if otel.Metrics != nil {
72+
otelMetricsEnabled = otel.Metrics.Enabled
73+
}
74+
}
75+
76+
// Process Prometheus configuration
77+
if telemetryConfig.Prometheus != nil {
78+
otelEnablePrometheusMetricsPath = telemetryConfig.Prometheus.Enabled
79+
}
80+
81+
return telemetry.MaybeMakeConfig(
82+
otelEndpoint,
83+
otelEnablePrometheusMetricsPath,
84+
otelTracingEnabled,
85+
otelMetricsEnabled,
86+
otelServiceName,
87+
otelSamplingRate,
88+
otelHeaders,
89+
otelInsecure,
90+
otelEnvironmentVariables,
91+
)
92+
}

‎cmd/thv/app/run_flags.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ func configureMiddlewareAndOptions(
549549
runFlags.ThvCABundle, runFlags.JWKSAuthTokenFile, runFlags.ResourceURL,
550550
runFlags.JWKSAllowPrivateIP, runFlags.InsecureAllowHTTP,
551551
),
552-
runner.WithTelemetryConfig(finalOtelEndpoint, runFlags.OtelEnablePrometheusMetricsPath,
552+
runner.WithTelemetryConfigFromFlags(finalOtelEndpoint, runFlags.OtelEnablePrometheusMetricsPath,
553553
runFlags.OtelTracingEnabled, runFlags.OtelMetricsEnabled, runFlags.OtelServiceName,
554554
finalOtelSamplingRate, runFlags.OtelHeaders, runFlags.OtelInsecure, finalOtelEnvironmentVariables,
555555
),

‎pkg/api/v1/workload_service.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ func (s *WorkloadService) BuildFullRunConfig(ctx context.Context, req *createReq
244244
"", "", "", "", "", false, false),
245245
runner.WithToolsFilter(req.ToolsFilter),
246246
runner.WithToolsOverride(toolsOverride),
247-
runner.WithTelemetryConfig("", false, false, false, "", 0.0, nil, false, nil),
247+
runner.WithTelemetryConfigFromFlags("", false, false, false, "", 0.0, nil, false, nil),
248248
}
249249

250250
// Determine transport type

‎pkg/runner/config_builder.go‎

Lines changed: 19 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,8 @@ func WithTokenExchangeConfig(config *tokenexchange.Config) RunConfigBuilderOptio
365365
}
366366
}
367367

368-
// WithTelemetryConfig configures telemetry settings (legacy - custom attributes handled via middleware)
369-
func WithTelemetryConfig(
368+
// WithTelemetryConfigFromFlags configures telemetry settings (legacy - custom attributes handled via middleware)
369+
func WithTelemetryConfigFromFlags(
370370
otelEndpoint string,
371371
otelEnablePrometheusMetricsPath bool,
372372
otelTracingEnabled bool,
@@ -377,51 +377,24 @@ func WithTelemetryConfig(
377377
otelInsecure bool,
378378
otelEnvironmentVariables []string,
379379
) RunConfigBuilderOption {
380-
return func(b *runConfigBuilder) error {
381-
if otelEndpoint == "" && !otelEnablePrometheusMetricsPath {
382-
return nil
383-
}
384-
385-
// Parse headers from key=value format
386-
headers := make(map[string]string)
387-
for _, header := range otelHeaders {
388-
parts := strings.SplitN(header, "=", 2)
389-
if len(parts) == 2 {
390-
headers[parts[0]] = parts[1]
391-
}
392-
}
393-
394-
// Use provided service name or default
395-
serviceName := otelServiceName
396-
if serviceName == "" {
397-
serviceName = telemetry.DefaultConfig().ServiceName
398-
}
399-
400-
// Process environment variables - split comma-separated values
401-
var processedEnvVars []string
402-
for _, envVarEntry := range otelEnvironmentVariables {
403-
// Split by comma and trim whitespace
404-
envVars := strings.Split(envVarEntry, ",")
405-
for _, envVar := range envVars {
406-
trimmed := strings.TrimSpace(envVar)
407-
if trimmed != "" {
408-
processedEnvVars = append(processedEnvVars, trimmed)
409-
}
410-
}
411-
}
380+
config := telemetry.MaybeMakeConfig(
381+
otelEndpoint,
382+
otelEnablePrometheusMetricsPath,
383+
otelTracingEnabled,
384+
otelMetricsEnabled,
385+
otelServiceName,
386+
otelSamplingRate,
387+
otelHeaders,
388+
otelInsecure,
389+
otelEnvironmentVariables,
390+
)
391+
return WithTelemetryConfig(config)
392+
}
412393

413-
b.config.TelemetryConfig = &telemetry.Config{
414-
Endpoint: otelEndpoint,
415-
ServiceName: serviceName,
416-
ServiceVersion: telemetry.DefaultConfig().ServiceVersion,
417-
TracingEnabled: otelTracingEnabled,
418-
MetricsEnabled: otelMetricsEnabled,
419-
SamplingRate: otelSamplingRate,
420-
Headers: headers,
421-
Insecure: otelInsecure,
422-
EnablePrometheusMetricsPath: otelEnablePrometheusMetricsPath,
423-
EnvironmentVariables: processedEnvVars,
424-
}
394+
// WithTelemetryConfig sets the telemetry configuration
395+
func WithTelemetryConfig(config *telemetry.Config) RunConfigBuilderOption {
396+
return func(b *runConfigBuilder) error {
397+
b.config.TelemetryConfig = config
425398
return nil
426399
}
427400
}

‎pkg/runner/config_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,7 @@ func TestRunConfigBuilder(t *testing.T) {
613613
WithLabels(nil),
614614
WithGroup(""),
615615
WithOIDCConfig(oidcIssuer, oidcAudience, oidcJwksURL, "", oidcClientID, "", "", "", "", false, false),
616-
WithTelemetryConfig("", false, false, false, "", 0.1, nil, false, nil),
616+
WithTelemetryConfigFromFlags("", false, false, false, "", 0.1, nil, false, nil),
617617
WithToolsFilter(nil),
618618
WithIgnoreConfig(&ignore.Config{
619619
LoadGlobal: false,
@@ -861,7 +861,7 @@ func TestRunConfigBuilder_MetadataOverrides(t *testing.T) {
861861
WithLabels(nil),
862862
WithGroup(""),
863863
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
864-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
864+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
865865
WithToolsFilter(nil),
866866
WithIgnoreConfig(&ignore.Config{
867867
LoadGlobal: false,
@@ -908,7 +908,7 @@ func TestRunConfigBuilder_EnvironmentVariableTransportDependency(t *testing.T) {
908908
WithLabels(nil),
909909
WithGroup(""),
910910
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
911-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
911+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
912912
WithToolsFilter(nil),
913913
WithIgnoreConfig(&ignore.Config{
914914
LoadGlobal: false,
@@ -961,7 +961,7 @@ func TestRunConfigBuilder_CmdArgsMetadataOverride(t *testing.T) {
961961
WithLabels(nil),
962962
WithGroup(""),
963963
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
964-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
964+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
965965
WithToolsFilter(nil),
966966
WithIgnoreConfig(&ignore.Config{
967967
LoadGlobal: false,
@@ -1016,7 +1016,7 @@ func TestRunConfigBuilder_CmdArgsMetadataDefaults(t *testing.T) {
10161016
WithLabels(nil),
10171017
WithGroup(""),
10181018
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
1019-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
1019+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
10201020
WithToolsFilter(nil),
10211021
WithIgnoreConfig(&ignore.Config{
10221022
LoadGlobal: false,
@@ -1067,7 +1067,7 @@ func TestRunConfigBuilder_VolumeProcessing(t *testing.T) {
10671067
WithLabels(nil),
10681068
WithGroup(""),
10691069
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
1070-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
1070+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
10711071
WithToolsFilter(nil),
10721072
WithIgnoreConfig(&ignore.Config{
10731073
LoadGlobal: false,
@@ -1140,7 +1140,7 @@ func TestRunConfigBuilder_FilesystemMCPScenario(t *testing.T) {
11401140
WithLabels(nil),
11411141
WithGroup(""),
11421142
WithOIDCConfig("", "", "", "", "", "", "", "", "", false, false),
1143-
WithTelemetryConfig("", false, false, false, "", 0, nil, false, nil),
1143+
WithTelemetryConfigFromFlags("", false, false, false, "", 0, nil, false, nil),
11441144
WithToolsFilter(nil),
11451145
WithIgnoreConfig(&ignore.Config{
11461146
LoadGlobal: false,

0 commit comments

Comments
 (0)