Skip to content

fix: add validation for malformed headers in metrics module and add unit tests #383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions otlp/otlpmetrics/cli.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package otlpmetrics

import (
"strings"
"time"

"github.com/spf13/cobra"
Expand All @@ -19,6 +20,7 @@ const (
OtelMetricsExporterOTLPModeFlag = "otel-metrics-exporter-otlp-mode"
OtelMetricsExporterOTLPEndpointFlag = "otel-metrics-exporter-otlp-endpoint"
OtelMetricsExporterOTLPInsecureFlag = "otel-metrics-exporter-otlp-insecure"
OtelMetricsExporterOTLPHeadersFlag = "otel-metrics-exporter-otlp-headers"
)

func AddFlags(flags *flag.FlagSet) {
Expand All @@ -31,6 +33,7 @@ func AddFlags(flags *flag.FlagSet) {
flags.String(OtelMetricsExporterOTLPModeFlag, "grpc", "OpenTelemetry metrics OTLP exporter mode (grpc|http)")
flags.String(OtelMetricsExporterOTLPEndpointFlag, "", "OpenTelemetry metrics grpc endpoint")
flags.Bool(OtelMetricsExporterOTLPInsecureFlag, false, "OpenTelemetry metrics grpc insecure")
flags.StringSlice(OtelMetricsExporterOTLPHeadersFlag, nil, "OpenTelemetry metrics grpc headers")

// notes(gfyrag): apps are in charge of exposing in memory metrics using whatever protocol it wants to
flags.Bool(OtelMetricsKeepInMemoryFlag, false, "Allow to keep metrics in memory")
Expand All @@ -45,12 +48,23 @@ func FXModuleFromFlags(cmd *cobra.Command) fx.Option {
otelMetricsRuntimeMinimumReadMemStatsInterval, _ := cmd.Flags().GetDuration(OtelMetricsRuntimeMinimumReadMemStatsIntervalFlag)
otelMetricsExporterPushInterval, _ := cmd.Flags().GetDuration(OtelMetricsExporterPushIntervalFlag)
otelMetricsKeepInMemory, _ := cmd.Flags().GetBool(OtelMetricsKeepInMemoryFlag)
otelMetricsExporterOTLPHeaders, _ := cmd.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag)
headersMap := make(map[string]string)
for _, header := range otelMetricsExporterOTLPHeaders {
parts := strings.SplitN(header, "=", 2)
if len(parts) == 2 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle the case where len(parts) == 1

headersMap[parts[0]] = parts[1]
} else if len(parts) == 1 && parts[0] != "" {
panic("malformed header: " + header + " (missing value)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn, just return a CLI error

}
}

return MetricsModule(ModuleConfig{
OTLPConfig: &OTLPConfig{
Mode: otelMetricsExporterOTLPMode,
Endpoint: otelMetricsExporterOTLPEndpoint,
Insecure: otelMetricsExporterOTLPInsecure,
Headers: headersMap,
},
Exporter: otelMetricsExporter,
RuntimeMetrics: otelMetricsRuntime,
Expand Down
43 changes: 43 additions & 0 deletions otlp/otlpmetrics/cli_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package otlpmetrics

import (
"testing"

"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
)

func TestHeadersParsing(t *testing.T) {
cmd := &cobra.Command{}
AddFlags(cmd.Flags())

cmd.SetArgs([]string{
"--otel-metrics-exporter-otlp-headers=Authorization=Bearer token,Content-Type=application/json",
})
err := cmd.Execute()
assert.NoError(t, err)

headers, _ := cmd.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag)
assert.Len(t, headers, 2)

cmd2 := &cobra.Command{}
AddFlags(cmd2.Flags())
cmd2.SetArgs([]string{
"--otel-metrics-exporter-otlp-headers=Authorization=Bearer token,BadHeader",
})
err = cmd2.Execute()
assert.NoError(t, err)

headers, _ = cmd2.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag)

cmd3 := &cobra.Command{}
AddFlags(cmd3.Flags())
cmd3.SetArgs([]string{
"--otel-metrics-exporter-otlp-headers=Authorization=Bearer token,BadHeader",
"--otel-metrics-exporter-otlp-mode=grpc",
})
err = cmd3.Execute()
assert.NoError(t, err)

_ = FXModuleFromFlags(cmd3)
}
11 changes: 11 additions & 0 deletions otlp/otlpmetrics/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type OTLPConfig struct {
Mode string
Endpoint string
Insecure bool
Headers map[string]string
}

func ProvideMetricsProviderOption(v any, annotations ...fx.Annotation) fx.Option {
Expand Down Expand Up @@ -132,6 +133,11 @@ func MetricsModule(cfg ModuleConfig) fx.Option {
return otlpmetricgrpc.WithInsecure()
}))
}
if cfg.OTLPConfig.Headers != nil {
options = append(options, ProvideOTLPMetricsGRPCOption(func() otlpmetricgrpc.Option {
return otlpmetricgrpc.WithHeaders(cfg.OTLPConfig.Headers)
}))
}
}

options = append(options, ProvideOTLPMetricsGRPCExporter())
Expand All @@ -147,6 +153,11 @@ func MetricsModule(cfg ModuleConfig) fx.Option {
return otlpmetrichttp.WithInsecure()
}))
}
if cfg.OTLPConfig.Headers != nil {
options = append(options, ProvideOTLPMetricsHTTPOption(func() otlpmetrichttp.Option {
return otlpmetrichttp.WithHeaders(cfg.OTLPConfig.Headers)
}))
}
}

options = append(options, ProvideOTLPMetricsHTTPExporter())
Expand Down
14 changes: 14 additions & 0 deletions otlp/otlptraces/cli.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package otlptraces

import (
"strings"

"github.com/formancehq/go-libs/v2/otlp"
"github.com/spf13/cobra"
flag "github.com/spf13/pflag"
Expand All @@ -16,6 +18,7 @@ const (
OtelTracesExporterOTLPModeFlag = "otel-traces-exporter-otlp-mode"
OtelTracesExporterOTLPEndpointFlag = "otel-traces-exporter-otlp-endpoint"
OtelTracesExporterOTLPInsecureFlag = "otel-traces-exporter-otlp-insecure"
OtelTracesExporterOTLPHeadersFlag = "otel-traces-exporter-otlp-headers"
)

func AddFlags(flags *flag.FlagSet) {
Expand All @@ -29,6 +32,7 @@ func AddFlags(flags *flag.FlagSet) {
flags.String(OtelTracesExporterOTLPModeFlag, "grpc", "OpenTelemetry traces OTLP exporter mode (grpc|http)")
flags.String(OtelTracesExporterOTLPEndpointFlag, "", "OpenTelemetry traces grpc endpoint")
flags.Bool(OtelTracesExporterOTLPInsecureFlag, false, "OpenTelemetry traces grpc insecure")
flags.StringSlice(OtelTracesExporterOTLPHeadersFlag, nil, "OpenTelemetry traces grpc headers")
}

func FXModuleFromFlags(cmd *cobra.Command) fx.Option {
Expand All @@ -47,11 +51,21 @@ func FXModuleFromFlags(cmd *cobra.Command) fx.Option {
mode, _ := cmd.Flags().GetString(OtelTracesExporterOTLPModeFlag)
endpoint, _ := cmd.Flags().GetString(OtelTracesExporterOTLPEndpointFlag)
insecure, _ := cmd.Flags().GetBool(OtelTracesExporterOTLPInsecureFlag)
headers, _ := cmd.Flags().GetStringSlice(OtelTracesExporterOTLPHeadersFlag)

headersMap := make(map[string]string)
for _, header := range headers {
parts := strings.SplitN(header, "=", 2)
if len(parts) == 2 {
headersMap[parts[0]] = parts[1]
}
}

return &OTLPConfig{
Mode: mode,
Endpoint: endpoint,
Insecure: insecure,
Headers: headersMap,
}
}(),
ServiceName: serviceName,
Expand Down
11 changes: 11 additions & 0 deletions otlp/otlptraces/traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type OTLPConfig struct {
Mode string
Endpoint string
Insecure bool
Headers map[string]string
}

type ModuleConfig struct {
Expand Down Expand Up @@ -109,6 +110,11 @@ func TracesModule(cfg ModuleConfig) fx.Option {
return otlptracegrpc.WithInsecure()
}))
}
if cfg.OTLPConfig.Headers != nil {
options = append(options, ProvideOTLPTracerGRPCClientOption(func() otlptracegrpc.Option {
return otlptracegrpc.WithHeaders(cfg.OTLPConfig.Headers)
}))
}
case otlp.ModeHTTP:
if cfg.OTLPConfig.Endpoint != "" {
options = append(options, ProvideOTLPTracerHTTPClientOption(func() otlptracehttp.Option {
Expand All @@ -120,6 +126,11 @@ func TracesModule(cfg ModuleConfig) fx.Option {
return otlptracehttp.WithInsecure()
}))
}
if cfg.OTLPConfig.Headers != nil {
options = append(options, ProvideOTLPTracerHTTPClientOption(func() otlptracehttp.Option {
return otlptracehttp.WithHeaders(cfg.OTLPConfig.Headers)
}))
}
}
}
switch mode {
Expand Down
Loading