fix(highlight-go): allow histogram bucket boundaries, and stop racing on the instrument cache

[Vadman97]

Summary

Follow-up to launchdarkly/observability#1780. Investigating why otel.ingest.timestamp_skew_sec was unusable in ClickHouse bottomed out in two defects here.

1. Instruments could not be configured. RecordHistogram called defaultMeter.Float64Histogram(name) with no options, so every histogram inherited OTEL's default boundaries — [0, 5, …, 10000], shaped for millisecond latencies. A metric on any other scale collapses entirely into the +Inf overflow bucket and no quantile is computable. The metric that prompted this only ever emits values ≥7200 (it measures a ±2h clamp), so 94% of its observations sat in the overflow bucket and 13 of its 16 buckets were empty by construction. Unit and description were unreachable for the same reason, so it exported with an empty unit despite a _sec name.

Each Record* function now has a WithOptions variant taking the matching OTEL instrument options, wired through hmetric too since that's the wrapper callers actually use:

var skewBuckets = []metric.Float64HistogramOption{
    metric.WithExplicitBucketBoundaries(7200, 10800, 21600, 43200, 86400, 604800),
    metric.WithUnit("s"),
    metric.WithDescription("Distance between an ingested timestamp and server time."),
}

hmetric.HistogramWithOptions(ctx, "otel.ingest.timestamp_skew_sec", skew, attrs, 1, skewBuckets...)

Options are read when the instrument is first built for a name and ignored afterwards — that's inherent to memoizing instruments, and the doc comments say so.

2. The instrument caches were read without a lock. Every Record* function took the mutex to build the instrument, released it, then indexed the map again to record:

float64HistogramsLock.Unlock()
...
float64Histograms[name].Record(metricCtx, value, ...)  // no lock held

That's an unsynchronized map read against a concurrent writer. Go can escalate it from a data race to a fatal concurrent map read and map write, which takes down the process — in a telemetry SDK, on a path any instrumented service hits under load.

The three copy-pasted blocks are now one generic instrumentCache that returns the instrument it looked up, with proper double-checked locking. Instruments stay lazily built: defaultMeter is a noop until StartOTLP installs the real provider, so building eagerly would permanently cache an instrument that discards every measurement.

Also: replaced the fmt.Printf calls with the package logger — a library shouldn't write to stdout — and fixed the counter path reporting its failures as "float64 histogram".

Compatibility

Additive. RecordMetric, RecordHistogram, RecordCount and the three hmetric wrappers keep their signatures and now delegate to the WithOptions variants.

How did you test this change?

go test ./... -race -count=1 in sdk/highlight-go. New otel_metrics_test.go drives the real sdkmetric pipeline through a ManualReader and asserts on what would actually be exported:

• default boundaries send everything above 10000 to the +Inf bucket (documents the problem)
• explicit boundaries are honored, with observations landing in the expected buckets
• unit and description reach the exported metric, for all three instrument kinds
• first call decides configuration, and later differing options don't drop measurements
• 192 goroutines racing on first use of 24 metric names across all three caches

I verified that last test is meaningful by restoring the old unsynchronized access pattern and re-running it: WARNING: DATA RACE ×6 and a failure. With the fix, clean.

TestParseConsoleMessages in sdk/highlight-go/log fails, but it fails identically on unmodified origin/main — a JSON type mismatch in the console-message parser, unrelated to this change.

Follow-up

Once released and the submodule is bumped, the observability backend can declare boundaries for otel.ingest.timestamp_skew_sec and a unit/description for otel.ingest.timestamp_clamped.

🤖 Generated with Claude Code

---

Note

Overview
Adds *WithOptions helpers in highlight-go and hmetric so histograms, counters, and gauges can be created with OTEL options (explicit bucket boundaries, unit, description). Existing Record* / Histogram / Incr / Gauge APIs are unchanged and delegate to those variants.

Replaces the three per-instrument maps and hand-rolled locking with a generic instrumentCache that uses double-checked locking and returns the built instrument from get, fixing unsynchronized map reads after unlock that could race under concurrent first use. Instrument creation errors now go through the package logger instead of fmt.Printf.

New otel_metrics_test.go exercises default vs explicit histogram bounds, metadata on export, first-call-wins configuration, and concurrent first-use safety via a ManualReader.

^{Reviewed by Cursor Bugbot for commit 394106c. Bugbot is set up for automated code reviews on this repo. Configure here.}