Skip to content
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

Creating repeated Float64ObservableGauge with the same name, but different callbacks the results in only one callback being used #5380

Open
Quinn-With-Two-Ns opened this issue May 19, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@Quinn-With-Two-Ns
Copy link

Description

When creating multiple Float64ObservableGauge with the same name, but different callbacks, it appears like only the first callback is used. This didn't used to be the case I tested in v1.21.0 and in that version both callbacks are called, the behaviour is also different if the callbacks are given as options when creating the gauge vs registering later which is very confusing.

I would like to understand:

  1. Was this an intentional breaking change? I couldn't find anything in the release notes, but maybe I missed it.
  2. Can I rely on the 2nd test behaviour or could that change as well?

Environment

  • OS: Mac OS
  • Architecture: ARM
  • Go Version: 1.21.1
  • opentelemetry-go version:v1.21.0 and v1.26.0

Steps To Reproduce

I created two tests in v1.21.0 both these test pass, in version v1.23.0 and v1.26.0 the first test fails since only the first callback is registered

Note: it is possible the behaviour changes in v1.22.0, I couldn't get this version using go get for some reason

package oteltest

import (
	"context"
	"testing"

	"go.opentelemetry.io/otel/attribute"
	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
	"go.opentelemetry.io/otel/sdk/metric/metricdata"
	"go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest"

	"github.com/stretchr/testify/assert"
	"go.opentelemetry.io/otel/metric"
)

func TestRepeatedObservableGaugesWithTheSameName(t *testing.T) {
	ctx := context.Background()
	metricReader := sdkmetric.NewManualReader()
	meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(metricReader))
	meter := meterProvider.Meter("test")

	_, err := meter.Float64ObservableGauge("test-metric-name",
		metric.WithFloat64Callback(func(ctx context.Context, o metric.Float64Observer) error {
			o.Observe(1.0, metric.WithAttributeSet(attribute.NewSet(attribute.String("tag1", "value1"))))
			return nil
		}))
	assert.NoError(t, err)

	_, err = meter.Float64ObservableGauge("test-metric-name",
		metric.WithFloat64Callback(func(ctx context.Context, o metric.Float64Observer) error {
			o.Observe(5.0, metric.WithAttributeSet(attribute.NewSet(attribute.String("tag1", "value2"))))
			return nil
		}))
	assert.NoError(t, err)

	// Assert result
	var rm metricdata.ResourceMetrics
	metricReader.Collect(ctx, &rm)
	assert.Len(t, rm.ScopeMetrics, 1)
	metrics := rm.ScopeMetrics[0].Metrics
	assert.Len(t, metrics, 1)
	want := metricdata.Metrics{
		Name: "test-metric-name",
		Data: metricdata.Gauge[float64]{
			DataPoints: []metricdata.DataPoint[float64]{
				{
					Value:      1.0,
					Attributes: attribute.NewSet(attribute.String("tag1", "value1")),
				},
				{
					Value:      5.0,
					Attributes: attribute.NewSet(attribute.String("tag1", "value2")),
				},
			},
		},
	}
	metricdatatest.AssertEqual(t, want, metrics[0], metricdatatest.IgnoreTimestamp())
}

func TestRepeatedObservableGaugesWithTheSameNameAndCallbacks(t *testing.T) {
	ctx := context.Background()
	metricReader := sdkmetric.NewManualReader()
	meterProvider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(metricReader))
	meter := meterProvider.Meter("test")

	og1, err := meter.Float64ObservableGauge("test-metric-name")
	assert.NoError(t, err)

	_, err = meter.RegisterCallback(func(ctx context.Context, o metric.Observer) error {
		o.ObserveFloat64(og1, 1.0, metric.WithAttributeSet(attribute.NewSet(attribute.String("tag1", "value1"))))
		return nil
	}, og1)
	assert.NoError(t, err)

	og2, err := meter.Float64ObservableGauge("test-metric-name")
	assert.NoError(t, err)

	_, err = meter.RegisterCallback(func(ctx context.Context, o metric.Observer) error {
		o.ObserveFloat64(og2, 5.0, metric.WithAttributeSet(attribute.NewSet(attribute.String("tag1", "value2"))))
		return nil
	}, og2)
	assert.NoError(t, err)

	// Assert result
	var rm metricdata.ResourceMetrics
	metricReader.Collect(ctx, &rm)
	assert.Len(t, rm.ScopeMetrics, 1)
	metrics := rm.ScopeMetrics[0].Metrics
	assert.Len(t, metrics, 1)
	want := metricdata.Metrics{
		Name: "test-metric-name",
		Data: metricdata.Gauge[float64]{
			DataPoints: []metricdata.DataPoint[float64]{
				{
					Value:      1.0,
					Attributes: attribute.NewSet(attribute.String("tag1", "value1")),
				},
				{
					Value:      5.0,
					Attributes: attribute.NewSet(attribute.String("tag1", "value2")),
				},
			},
		},
	}
	metricdatatest.AssertEqual(t, want, metrics[0], metricdatatest.IgnoreTimestamp())
}

Expected behavior

I would expect both tests to pass. Since the spec says

Every currently registered Callback associated with a set of instruments MUST be evaluated exactly once during collection prior to reading data for that instrument set.

And I don't see anything in the spec as to why the 2nd callback should not be registered just because a gauge with the same name already exists. Did I miss something?

@Quinn-With-Two-Ns Quinn-With-Two-Ns added the bug Something isn't working label May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant