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

Default view should not be required when other views passed #3224

Closed
Tracked by #2796
MrAlias opened this issue Sep 22, 2022 · 1 comment · Fixed by #3237
Closed
Tracked by #2796

Default view should not be required when other views passed #3224

MrAlias opened this issue Sep 22, 2022 · 1 comment · Fixed by #3237
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working pkg:SDK Related to an SDK package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Sep 22, 2022

Description

If a user passes a view to the MeterProvider during creation and an instrument not matching that view measures values it is not exported. A user is required to pass the default view as well as their custom view to ensure the default behavior on unmatched views.

This is contrary to the specification:

  • If the MeterProvider has one or more View(s) registered:
    • [...]
    • If the Instrument could not match with any of the registered View(s), the
      SDK SHOULD enable the instrument using the default aggregation and temporality.
      Users can configure match-all Views using Drop aggregation
      to disable instruments by default.

Environment

  • Go Version: 1.19
  • opentelemetry-go version: 35019d3

Steps To Reproduce

main.go:

package main

import (
	"context"
	"fmt"
	"log"
	"net/http"
	"os"
	"os/signal"

	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/client_golang/prometheus/promhttp"

	otelprom "go.opentelemetry.io/otel/exporters/prometheus"
	"go.opentelemetry.io/otel/sdk/metric"
	"go.opentelemetry.io/otel/sdk/metric/view"
)

func main() {
	ctx := context.Background()

	exporter := otelprom.New()
	v, err := view.New(
		view.MatchInstrumentName("unmatched-name"),
		view.WithRename("bar"),
	)
	if err != nil {
		log.Fatal(err)
	}

	provider := metric.NewMeterProvider(metric.WithReader(exporter, v))
	meter := provider.Meter("example")

	go serveMetrics(exporter.Collector)

	counter, err := meter.SyncFloat64().Counter("foo")
	if err != nil {
		log.Fatal(err)
	}
	counter.Add(ctx, 5)

	ctx, _ = signal.NotifyContext(ctx, os.Interrupt)
	<-ctx.Done()
}

func serveMetrics(collector prometheus.Collector) {
	registry := prometheus.NewRegistry()
	err := registry.Register(collector)
	if err != nil {
		fmt.Printf("error registering collector: %v", err)
		return
	}

	log.Printf("serving metrics at localhost:2222/metrics")
	http.Handle("/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{}))
	err = http.ListenAndServe(":2222", nil)
	if err != nil {
		fmt.Printf("error serving http: %v", err)
		return
	}
}

go.mod:

go 1.18

require (
	github.com/prometheus/client_golang v1.13.0
	go.opentelemetry.io/otel v1.10.0
	go.opentelemetry.io/otel/exporters/prometheus v0.31.0
	go.opentelemetry.io/otel/metric v0.32.0
	go.opentelemetry.io/otel/sdk v1.10.0
	go.opentelemetry.io/otel/sdk/metric v0.32.0
)

require (
	github.com/beorn7/perks v1.0.1 // indirect
	github.com/cespare/xxhash/v2 v2.1.2 // indirect
	github.com/go-logr/logr v1.2.3 // indirect
	github.com/go-logr/stdr v1.2.2 // indirect
	github.com/golang/protobuf v1.5.2 // indirect
	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
	github.com/prometheus/client_model v0.2.0 // indirect
	github.com/prometheus/common v0.37.0 // indirect
	github.com/prometheus/procfs v0.8.0 // indirect
	go.opentelemetry.io/otel/trace v1.10.0 // indirect
	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
	google.golang.org/protobuf v1.28.1 // indirect
)
$ go run .
2022/09/22 09:08:00 serving metrics at localhost:2222/metrics
# Returns nothing.
$ curl localhost:2222/metrics

Expected behavior

$ curl localhost:2222/metrics
# HELP foo
# TYPE foo counter
foo 5
@MrAlias MrAlias added bug Something isn't working pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Sep 22, 2022
@MrAlias MrAlias added this to the Metric v0.32.1 milestone Sep 22, 2022
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue Sep 27, 2022
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue Sep 27, 2022
@MrAlias MrAlias self-assigned this Sep 27, 2022
@MrAlias
Copy link
Contributor Author

MrAlias commented Sep 27, 2022

Verified the changes proposed in #3237 resolve this issue based on the provided example.

MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue Sep 28, 2022
Aneurysm9 pushed a commit that referenced this issue Oct 7, 2022
* Use default view if inst matches no other

Fix #3224

* Test default view applied if no match

* Add changes to changelog

* Remove unneeded views len check in WithReader

* Do not return agg if adding err-ed

* Revert "Do not return agg if adding err-ed"

This reverts commit b56efb0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant