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

SumObserver sums up observations #1368

Closed
riptl opened this issue Nov 25, 2020 · 1 comment · Fixed by #1381
Closed

SumObserver sums up observations #1368

riptl opened this issue Nov 25, 2020 · 1 comment · Fixed by #1381
Milestone

Comments

@riptl
Copy link

riptl commented Nov 25, 2020

otel version: v0.14.0

Expected behavior

The OpenTelemetry specification claims:

Asynchronous, adding instruments (e.g., SumObserver) are used to capture sums directly. This means, for example, that in any sequence of SumObserver observations for a given instrument and label set, the Last Value defines the sum of the instrument.

The comment on Int64SumObserver claims:

// Int64SumObserver is a metric that captures a precomputed sum of
// int64 values at a point in time.

I expect that calling Int64SumObserver.Observe(v) only returns the latest observation instead of a sum of all observations.

Actual Behavior

Int64SumObserver sums up all observations instead of using the last value.

This makes it impossible to define Prometheus COUNTERs from external sources.
I found this bug when trying to export a counter in Redis.

Code Example

package example

import (
	"context"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"
	"time"

	"github.com/stretchr/testify/require"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/exporters/metric/prometheus"
	"go.opentelemetry.io/otel/metric"
)

func TestSums(t *testing.T) {
	// Install new Prometheus exporter
	exporter, err := prometheus.InstallNewPipeline(prometheus.Config{})
	require.NoError(t, err)
	// Define a sum observer that always sees "100".
	_, err = otel.Meter("test").NewInt64SumObserver("test_sum", func(ctx context.Context, result metric.Int64ObserverResult) {
		result.Observe(100)
	})
	require.NoError(t, err)
	// Send some requests
	req := &http.Request{
		Method: "GET",
		URL: &url.URL{
			Scheme: "http",
			Host:   "localhost",
			Path:   "/metrics",
		},
	}
	exporter.ServeHTTP(httptest.NewRecorder(), req)
	time.Sleep(15*time.Second)
	exporter.ServeHTTP(httptest.NewRecorder(), req)
	time.Sleep(15*time.Second)
	exporter.ServeHTTP(httptest.NewRecorder(), req)
	time.Sleep(15*time.Second)
	// Print last request
	recorder := httptest.NewRecorder()
	exporter.ServeHTTP(recorder, req)
	body := recorder.Body.String()
	t.Log("Exported Metrics\n" + body)
}

Outputs

# HELP test_sum 
# TYPE test_sum counter
test_sum 400
@jmacd
Copy link
Contributor

jmacd commented Dec 8, 2020

#1381

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 a pull request may close this issue.

3 participants