Skip to content

Conversation

@Arunodoy18
Copy link

Fixes #14326

mdatagen was generating invalid Go code for histogram metrics by using NumberDataPoint API methods (SetDoubleValue, ValueType) on HistogramDataPoint types, causing compilation errors.

Changes

metrics.go.tmpl:

  • Added conditional logic to detect histogram metrics: {{- if eq $metric.Data.Type "Histogram" }}
  • For histograms, use HistogramDataPoint API:
    • dp.SetCount(1) and dp.SetSum(val) instead of dp.SetDoubleValue(val)
    • dp.ExplicitBounds().FromRaw() and dp.BucketCounts().FromRaw() for bucket setup
  • Maintains backward compatibility for Sum/Gauge metrics

metrics_test.go.tmpl:

  • Added histogram-specific test assertions
  • Use assert.Equal(uint64(1), dp.Count()) and assert.InDelta(float64(1), dp.Sum()) for histograms
  • Use existing NumberDataPoint assertions for Sum/Gauge metrics

metadata.yaml:

  • Added system.cpu.utilization histogram metric to samplescraper for validation

Testing

  • ✅ All existing tests pass
  • ✅ New histogram metric generates valid Go code
  • ✅ Generated code compiles successfully: go build ./...
  • ✅ Histogram tests pass: go test ./...

Backward Compatibility

  • ✅ No changes to existing Sum/Gauge metric generation
  • ✅ No changes to metadata.yaml format
  • ✅ Template uses conditional logic to isolate histogram handling

Example Usage

metrics:
  system.memory.usage:
    enabled: true
    description: Memory usage histogram
    unit: By
    histogram:
      value_type: double
      bucket_boundaries: [0, 1024, 2048, 4096, 8192]
    attributes: [state]

Generated code will correctly use HistogramDataPoint API methods.

When service.telemetry.metrics.level is set to 'none', the collector
should skip registering process metrics to avoid errors on platforms
where gopsutil is not supported (such as AIX).

This change conditionally registers process metrics only when the
metrics level is not LevelNone, preventing the 'failed to register
process metrics: not implemented yet' error on unsupported platforms.

Fixes regression introduced in v0.136.0 where the check for metrics
level was removed.
Similar to the resolution for pcommon.Value in previous changes, this update
ensures consistent documentation across all pdata types by clarifying that
calling functions on zero-initialized instances is invalid usage.

Changes:
- Updated template files (one_of_field.go, one_of_message_value.go) to generate
  improved comment wording
- Updated pcommon/value.go comments manually
- Updated all generated pdata files to use consistent wording:
  'is invalid and will cause a panic' instead of 'will cause a panic'

This makes it clearer that using zero-initialized instances is not just
dangerous but explicitly invalid usage, improving API documentation clarity.
Fixes open-telemetry#14326

mdatagen was generating invalid Go code for histogram metrics by using NumberDataPoint API methods (SetDoubleValue, ValueType) on HistogramDataPoint types, causing compilation errors.

Changes: Update metrics.go.tmpl to use HistogramDataPoint API for histogram metrics with dp.SetCount(1) and dp.SetSum(val). Update metrics_test.go.tmpl to assert dp.Count() and dp.Sum() for histograms. Add system.cpu.utilization histogram to samplescraper metadata for validation.
@Arunodoy18 Arunodoy18 requested review from a team, bogdandrutu and dmitryax as code owners January 12, 2026 21:00
@@ -223,8 +235,11 @@ func New(ctx context.Context, set Settings, cfg Config) (_ *Service, resultErr e
return nil, err
}

if err := proctelemetry.RegisterProcessMetrics(srv.telemetrySettings); err != nil {
return nil, fmt.Errorf("failed to register process metrics: %w", err)
// Only register process metrics if metrics telemetry is enabled
Copy link
Member

Choose a reason for hiding this comment

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

This change seems unrelated. Could you keep your PRs atomic?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing this out 👍

You’re right — the process metrics change is unrelated to the mdatagen histogram fix.
I’ll split this into a separate PR and keep this one focused only on histogram generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mdatagen cannot generate histogram correctly

2 participants