Merge branch 'main' into getting-started-doc-fix

.github/workflows/benchmark.yml

@@ -22,7 +22,7 @@ jobs:
           path: ./benchmarks
           key: ${{ runner.os }}-benchmark
       - name: Store benchmarks result
-        uses: benchmark-action/github-action-benchmark@v1.13.0
+        uses: benchmark-action/github-action-benchmark@v1.14.0
         with:
           name: Benchmarks
           tool: 'go'

.github/workflows/ci.yml

@@ -95,7 +95,7 @@ jobs:
           cp coverage.txt $TEST_RESULTS
           cp coverage.html $TEST_RESULTS
       - name: Upload coverage report
-        uses: codecov/codecov-action@v3.1.0
+        uses: codecov/codecov-action@v3.1.1
         with:
           file: ./coverage.txt
           fail_ci_if_error: true

.github/workflows/links-fail-fast.yml

@@ -11,6 +11,6 @@ jobs:
       - uses: actions/checkout@v3
 
       - name: Link Checker
-        uses: lycheeverse/lychee-action@v1.4.1
+        uses: lycheeverse/lychee-action@v1.5.1
         with:
           fail: true

.github/workflows/links.yml

@@ -16,7 +16,7 @@ jobs:
 
     - name: Link Checker
       id: lychee
-      uses: lycheeverse/lychee-action@v1.4.1
+      uses: lycheeverse/lychee-action@v1.5.1
 
     - name: Create Issue From File
       if: steps.lychee.outputs.exit_code != 0

CHANGELOG.md

@@ -11,14 +11,18 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Added
 
 - Prometheus exporter will register with a prometheus registerer on creation, there are options to control this. (#3239)
+- Added the `WithAggregationSelector` option to the `go.opentelemetry.io/otel/exporters/prometheus` package to change the `AggregationSelector` used. (#3341)
 
 ### Changed
 
 - `sdktrace.TraceProvider.Shutdown` and `sdktrace.TraceProvider.ForceFlush` to not return error when no processor register. (#3268)
 - The `"go.opentelemetry.io/otel/exporters/prometheus".New` now also returns an error indicating the failure to register the exporter with Prometheus. (#3239)
+- The prometheus exporter will no longer try to enumerate the metrics it will send to prometheus on startup.
+   This fixes the `reader is not registered` warning currently emitted on startup. (#3291 #3342)
 
 ### Fixed
 
+- Fix function `baggage.NewMember` to decode the `value` parameter instead of directly use it according to the W3C specification. (#3226)
 - Slice attributes of `attribute` package are now comparable based on their value, not instance. (#3108 #3252)
 - Prometheus exporter will now cumulatively sum histogram buckets. (#3281)
 - Export the sum of each histogram datapoint uniquely with the `go.opentelemetry.io/otel/exporters/otlpmetric` exporters. (#3284, #3293)

baggage/baggage.go

@@ -250,8 +250,9 @@ type Member struct {
 	hasData bool
 }
 
-// NewMember returns a new Member from the passed arguments. An error is
-// returned if the created Member would be invalid according to the W3C
+// NewMember returns a new Member from the passed arguments. The key will be
+// used directly while the value will be url decoded after validation. An error
+// is returned if the created Member would be invalid according to the W3C
 // Baggage specification.
 func NewMember(key, value string, props ...Property) (Member, error) {
 	m := Member{
@@ -263,7 +264,11 @@ func NewMember(key, value string, props ...Property) (Member, error) {
 	if err := m.validate(); err != nil {
 		return newInvalidMember(), err
 	}
-
+	decodedValue, err := url.QueryUnescape(value)
+	if err != nil {
+		return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value)
+	}
+	m.value = decodedValue
 	return m, nil
 }
 
@@ -328,8 +333,9 @@ func parseMember(member string) (Member, error) {
 	return Member{key: key, value: value, properties: props, hasData: true}, nil
 }
 
-// validate ensures m conforms to the W3C Baggage specification, returning an
-// error otherwise.
+// validate ensures m conforms to the W3C Baggage specification.
+// A key is just an ASCII string, but a value must be URL encoded UTF-8,
+// returning an error otherwise.
 func (m Member) validate() error {
 	if !m.hasData {
 		return fmt.Errorf("%w: %q", errInvalidMember, m)

baggage/baggage_test.go

@@ -768,6 +768,23 @@ func TestNewMember(t *testing.T) {
 	}
 	assert.Equal(t, expected, m)
 
+	// wrong value with wrong decoding
+	val = "%zzzzz"
+	_, err = NewMember(key, val, p)
+	assert.ErrorIs(t, err, errInvalidValue)
+
+	// value should be decoded
+	val = "%3B"
+	m, err = NewMember(key, val, p)
+	expected = Member{
+		key:        key,
+		value:      ";",
+		properties: properties{{key: "foo", hasData: true}},
+		hasData:    true,
+	}
+	assert.NoError(t, err)
+	assert.Equal(t, expected, m)
+
 	// Ensure new member is immutable.
 	p.key = "bar"
 	assert.Equal(t, expected, m)
@@ -784,6 +801,17 @@ func TestPropertiesValidate(t *testing.T) {
 	assert.NoError(t, p.validate())
 }
 
+func TestMemberString(t *testing.T) {
+	// normal key value pair
+	member, _ := NewMember("key", "value")
+	memberStr := member.String()
+	assert.Equal(t, memberStr, "key=value")
+	// encoded key
+	member, _ = NewMember("key", "%3B")
+	memberStr = member.String()
+	assert.Equal(t, memberStr, "key=%3B")
+}
+
 var benchBaggage Baggage
 
 func BenchmarkNew(b *testing.B) {

exporters/prometheus/confg_test.go

@@ -19,29 +19,52 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
+
+	"go.opentelemetry.io/otel/sdk/metric"
+	"go.opentelemetry.io/otel/sdk/metric/aggregation"
+	"go.opentelemetry.io/otel/sdk/metric/view"
 )
 
 func TestNewConfig(t *testing.T) {
 	registry := prometheus.NewRegistry()
 
+	aggregationSelector := func(view.InstrumentKind) aggregation.Aggregation { return nil }
+
 	testCases := []struct {
-		name           string
-		options        []Option
-		wantRegisterer prometheus.Registerer
+		name            string
+		options         []Option
+		wantRegisterer  prometheus.Registerer
+		wantAggregation metric.AggregationSelector
 	}{
 		{
 			name:           "Default",
 			options:        nil,
 			wantRegisterer: prometheus.DefaultRegisterer,
 		},
-
 		{
 			name: "WithRegisterer",
 			options: []Option{
 				WithRegisterer(registry),
 			},
 			wantRegisterer: registry,
 		},
+		{
+			name: "WithAggregationSelector",
+			options: []Option{
+				WithAggregationSelector(aggregationSelector),
+			},
+			wantRegisterer:  prometheus.DefaultRegisterer,
+			wantAggregation: aggregationSelector,
+		},
+		{
+			name: "With Multiple Options",
+			options: []Option{
+				WithRegisterer(registry),
+				WithAggregationSelector(aggregationSelector),
+			},
+			wantRegisterer:  registry,
+			wantAggregation: aggregationSelector,
+		},
 		{
 			name: "nil options do nothing",
 			options: []Option{
@@ -58,3 +81,31 @@ func TestNewConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigManualReaderOptions(t *testing.T) {
+	aggregationSelector := func(view.InstrumentKind) aggregation.Aggregation { return nil }
+
+	testCases := []struct {
+		name            string
+		config          config
+		wantOptionCount int
+	}{
+		{
+			name:            "Default",
+			config:          config{},
+			wantOptionCount: 0,
+		},
+
+		{
+			name:            "WithAggregationSelector",
+			config:          config{aggregation: aggregationSelector},
+			wantOptionCount: 1,
+		},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			opts := tt.config.manualReaderOptions()
+			assert.Len(t, opts, tt.wantOptionCount)
+		})
+	}
+}

exporters/prometheus/config.go

@@ -16,11 +16,14 @@ package prometheus // import "go.opentelemetry.io/otel/exporters/prometheus"
 
 import (
 	"github.com/prometheus/client_golang/prometheus"
+
+	"go.opentelemetry.io/otel/sdk/metric"
 )
 
 // config contains options for the exporter.
 type config struct {
-	registerer prometheus.Registerer
+	registerer  prometheus.Registerer
+	aggregation metric.AggregationSelector
 }
 
 // newConfig creates a validated config configured with options.
@@ -37,6 +40,14 @@ func newConfig(opts ...Option) config {
 	return cfg
 }
 
+func (cfg config) manualReaderOptions() []metric.ManualReaderOption {
+	opts := []metric.ManualReaderOption{}
+	if cfg.aggregation != nil {
+		opts = append(opts, metric.WithAggregationSelector(cfg.aggregation))
+	}
+	return opts
+}
+
 // Option sets exporter option values.
 type Option interface {
 	apply(config) config
@@ -57,3 +68,13 @@ func WithRegisterer(reg prometheus.Registerer) Option {
 		return cfg
 	})
 }
+
+// WithAggregationSelector configure the Aggregation Selector the exporter will
+// use. If no AggregationSelector is provided the DefaultAggregationSelector is
+// used.
+func WithAggregationSelector(agg metric.AggregationSelector) Option {
+	return optionFunc(func(cfg config) config {
+		cfg.aggregation = agg
+		return cfg
+	})
+}

exporters/prometheus/exporter.go

@@ -50,7 +50,7 @@ func New(opts ...Option) (*Exporter, error) {
 	// this assumes that the default temporality selector will always return cumulative.
 	// we only support cumulative temporality, so building our own reader enforces this.
 	// TODO (#3244): Enable some way to configure the reader, but not change temporality.
-	reader := metric.NewManualReader()
+	reader := metric.NewManualReader(cfg.manualReaderOptions()...)
 
 	collector := &collector{
 		reader: reader,
@@ -69,13 +69,11 @@ func New(opts ...Option) (*Exporter, error) {
 
 // Describe implements prometheus.Collector.
 func (c *collector) Describe(ch chan<- *prometheus.Desc) {
-	metrics, err := c.reader.Collect(context.TODO())
-	if err != nil {
-		otel.Handle(err)
-	}
-	for _, metricData := range getMetricData(metrics) {
-		ch <- metricData.description
-	}
+	// The Opentelemetry SDK doesn't have information on which will exist when the collector
+	// is registered. By returning nothing we are an "unchecked" collector in Prometheus,
+	// and assume responsibility for consistency of the metrics produced.
+	//
+	// See https://pkg.go.dev/github.com/prometheus/client_golang@v1.13.0/prometheus#hdr-Custom_Collectors_and_constant_Metrics
 }
 
 // Collect implements prometheus.Collector.

internal/tools/go.mod

@@ -6,7 +6,7 @@ require (
 	github.com/client9/misspell v0.3.4
 	github.com/gogo/protobuf v1.3.2
 	github.com/golangci/golangci-lint v1.48.0
-	github.com/itchyny/gojq v0.12.7
+	github.com/itchyny/gojq v0.12.9
 	github.com/jcchavezs/porto v0.4.0
 	github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad
 	go.opentelemetry.io/build-tools/crosslink v0.0.0-20220706175322-58de0d25b85c
@@ -90,7 +90,7 @@ require (
 	github.com/hexops/gotextdiff v1.0.3 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
-	github.com/itchyny/timefmt-go v0.1.3 // indirect
+	github.com/itchyny/timefmt-go v0.1.4 // indirect
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
 	github.com/jgautheron/goconst v1.5.1 // indirect
 	github.com/jingyugao/rowserrcheck v1.1.1 // indirect
@@ -110,8 +110,8 @@ require (
 	github.com/maratori/testpackage v1.1.0 // indirect
 	github.com/matoous/godox v0.0.0-20210227103229-6504466cf951 // indirect
 	github.com/mattn/go-colorable v0.1.12 // indirect
-	github.com/mattn/go-isatty v0.0.14 // indirect
-	github.com/mattn/go-runewidth v0.0.9 // indirect
+	github.com/mattn/go-isatty v0.0.16 // indirect
+	github.com/mattn/go-runewidth v0.0.13 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
 	github.com/mbilski/exhaustivestruct v1.2.0 // indirect
 	github.com/mgechev/revive v1.2.1 // indirect
@@ -138,6 +138,7 @@ require (
 	github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 // indirect
 	github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95 // indirect
 	github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 // indirect
+	github.com/rivo/uniseg v0.2.0 // indirect
 	github.com/ryancurrah/gomodguard v1.2.4 // indirect
 	github.com/ryanrolds/sqlclosecheck v0.3.0 // indirect
 	github.com/sanposhiho/wastedassign/v2 v2.0.6 // indirect
@@ -184,7 +185,7 @@ require (
 	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
 	golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect
 	golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect
-	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
+	golang.org/x/sys v0.0.0-20220829200755-d48e67d00261 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/ini.v1 v1.66.6 // indirect