From dff20a2e5b9481f331b6f86061e88d815bc46120 Mon Sep 17 00:00:00 2001 From: Yousef Khan Date: Thu, 9 Jul 2026 15:54:09 -0700 Subject: [PATCH] Match Influx/Timescale logic formatting in OTEL Provider --- pkg/otel/otel.go | 32 ++++++--- pkg/otel/params_test.go | 143 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 pkg/otel/params_test.go diff --git a/pkg/otel/otel.go b/pkg/otel/otel.go index 9864f8e..37ceabe 100644 --- a/pkg/otel/otel.go +++ b/pkg/otel/otel.go @@ -25,11 +25,25 @@ var ( //go:embed schema.json var schema string +var uiSchema = `{"ui:order": ["endpoint", "serviceName", "events", "telemetry"]}` + type Params struct { Endpoint string `json:"endpoint"` ServiceName string `json:"serviceName"` - Events bool `json:"events"` - Telemetry bool `json:"telemetry"` + Events *bool `json:"events,omitempty"` + Telemetry *bool `json:"telemetry,omitempty"` +} + +// EventsEnabled reports whether events should be pushed. Profiles saved +// before this option existed have no key and must stay enabled. +func (p Params) EventsEnabled() bool { + return p.Events == nil || *p.Events +} + +// TelemetryEnabled reports whether telemetry should be pushed. Profiles +// saved before this option existed have no key and must stay enabled. +func (p Params) TelemetryEnabled() bool { + return p.Telemetry == nil || *p.Telemetry } type otelProvider struct{} @@ -38,8 +52,6 @@ func (o *otelProvider) Default() Params { return Params{ Endpoint: "localhost:4317", ServiceName: "hermes", - Events: true, - Telemetry: true, } } @@ -48,7 +60,7 @@ func (o *otelProvider) Start( settings Params, session host.ConnectSession, ) error { - if !settings.Events && !settings.Telemetry { + if !settings.EventsEnabled() && !settings.TelemetryEnabled() { return fmt.Errorf("at least one of events or telemetry must be enabled") } @@ -61,7 +73,7 @@ func (o *otelProvider) Start( return fmt.Errorf("failed to create OTEL resource: %w", err) } - if settings.Events { + if settings.EventsEnabled() { session.Log().Info("exporting events to OTEL collector") logOpts := []otlploggrpc.Option{otlploggrpc.WithInsecure()} @@ -88,9 +100,11 @@ func (o *otelProvider) Start( host.Event.On(ctx, func(msg *pb.SourcedEvent) { handler.Handle(context.Background(), msg.GetEvent().Record()) }) + } else { + session.Log().Info("event logging to OTEL collector is disabled by profile settings") } - if settings.Telemetry { + if settings.TelemetryEnabled() { session.Log().Info("exporting telemetry to OTEL collector") metricOpts := []otlpmetricgrpc.Option{otlpmetricgrpc.WithInsecure()} @@ -140,6 +154,8 @@ func (o *otelProvider) Start( cache <- m } }) + } else { + session.Log().Info("telemetry logging to OTEL collector is disabled by profile settings") } session.Started() @@ -153,7 +169,7 @@ func Init() error { "OpenTelemetry", &otelProvider{}, schema, - `{"ui:order": ["endpoint", "serviceName", "events", "telemetry"]}`, + uiSchema, ) return err } diff --git a/pkg/otel/params_test.go b/pkg/otel/params_test.go new file mode 100644 index 0000000..eddadea --- /dev/null +++ b/pkg/otel/params_test.go @@ -0,0 +1,143 @@ +package otel + +import ( + "encoding/json" + "testing" +) + +func TestParamsWithoutTogglesKeepsBothEnabled(t *testing.T) { + // Settings saved before the toggles existed must keep logging both. + var p Params + if err := json.Unmarshal([]byte(`{"endpoint":"localhost:4317","serviceName":"hermes"}`), &p); err != nil { + t.Fatalf("failed to decode settings: %v", err) + } + + if !p.EventsEnabled() { + t.Error("events should be enabled when the key is absent") + } + if !p.TelemetryEnabled() { + t.Error("telemetry should be enabled when the key is absent") + } +} + +func TestParamsCanDisableEventsOnly(t *testing.T) { + var p Params + if err := json.Unmarshal([]byte(`{"events":false,"telemetry":true}`), &p); err != nil { + t.Fatalf("failed to decode settings: %v", err) + } + + if p.EventsEnabled() { + t.Error("events should be disabled when set to false") + } + if !p.TelemetryEnabled() { + t.Error("telemetry should stay enabled when set to true") + } +} + +func TestParamsCanDisableTelemetryOnly(t *testing.T) { + var p Params + if err := json.Unmarshal([]byte(`{"events":true,"telemetry":false}`), &p); err != nil { + t.Fatalf("failed to decode settings: %v", err) + } + + if !p.EventsEnabled() { + t.Error("events should stay enabled when set to true") + } + if p.TelemetryEnabled() { + t.Error("telemetry should be disabled when set to false") + } +} + +func TestParamsCanDisableBoth(t *testing.T) { + var p Params + if err := json.Unmarshal([]byte(`{"events":false,"telemetry":false}`), &p); err != nil { + t.Fatalf("failed to decode settings: %v", err) + } + + if p.EventsEnabled() { + t.Error("events should be disabled when set to false") + } + if p.TelemetryEnabled() { + t.Error("telemetry should be disabled when set to false") + } +} + +func TestNilTogglesAreOmittedFromMarshaledSettings(t *testing.T) { + // The frontend renders settings JSON straight into the form. A literal + // null bypasses the schema's default:true and draws an unchecked box + // while the backend keeps logging — nil pointers must serialize as + // absent keys, not null. + b, err := json.Marshal((&otelProvider{}).Default()) + if err != nil { + t.Fatalf("failed to marshal default params: %v", err) + } + + var m map[string]json.RawMessage + if err := json.Unmarshal(b, &m); err != nil { + t.Fatalf("failed to decode marshaled params: %v", err) + } + + if v, ok := m["events"]; ok { + t.Errorf("unset events should be omitted from settings, got %s", v) + } + if v, ok := m["telemetry"]; ok { + t.Errorf("unset telemetry should be omitted from settings, got %s", v) + } +} + +func TestExplicitNullParsesAsNilAndEnablesLogging(t *testing.T) { + // Literal null in stored settings (perhaps from corrupted state or + // manual edit) must behave the same as absent. + var p Params + if err := json.Unmarshal([]byte(`{"events":null,"telemetry":null}`), &p); err != nil { + t.Fatalf("failed to decode settings: %v", err) + } + + if !p.EventsEnabled() { + t.Error("events should be enabled when the key is null") + } + if !p.TelemetryEnabled() { + t.Error("telemetry should be enabled when the key is null") + } +} + +func TestSchemaAndUiOrderStayInSync(t *testing.T) { + // Every key in schema.json properties must appear in ui:order, and vice + // versa. If we add a field but forget to update the ui:order list, the + // form renders it at the end and looks broken. + var sch struct { + Properties map[string]any `json:"properties"` + } + if err := json.Unmarshal([]byte(schema), &sch); err != nil { + t.Fatalf("failed to decode schema.json: %v", err) + } + + var uiOrder struct { + Order []string `json:"ui:order"` + } + if err := json.Unmarshal([]byte(uiSchema), &uiOrder); err != nil { + t.Fatalf("failed to decode uiSchema: %v", err) + } + + schemaKeys := make(map[string]bool) + for k := range sch.Properties { + schemaKeys[k] = true + } + + orderKeys := make(map[string]bool) + for _, k := range uiOrder.Order { + orderKeys[k] = true + } + + for k := range schemaKeys { + if !orderKeys[k] { + t.Errorf("schema.json property %q is missing from ui:order", k) + } + } + + for k := range orderKeys { + if !schemaKeys[k] { + t.Errorf("ui:order lists %q but schema.json has no such property", k) + } + } +}