update comment w/ issue number

Signed-off-by: Alex Boten <aboten@lightstep.com>

.chloggen/codeboten_may-29-otlp-export.yaml

@@ -1,21 +1,20 @@
 # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
-change_type: enhancement
+change_type: breaking
 
 # The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
 component: service
 
 # A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
-note: "add ability to configure prometheus export via `readers`"
+note: Enable configuration of collector telemetry through prometheus reader
 
 # One or more tracking issues or pull requests related to the change
-issues: [7641]
+issues: []
 
 # (Optional) One or more lines of additional information to render under the primary note.
 # These lines will be padded with 2 spaces and then inserted directly into the document.
 # Use pipe (|) for multiline entries.
 subtext: |
-  This enables end users to configure additional prometheus exporters for the collector's telemetry
-  via the `readers` configuration option. Configuring prometheus through the existing method
-  of setting the service::metrics::address will continue to work, and only log a warning for users
-  who have enabled the `telemetry.useOtelWithSDKConfigurationForInternalTelemetry` feature gate.
-
+  These options are still experimental. To enable them, users must enable both
+  `telemetry.useOtelForInternalMetrics` and `telemetry.useOtelWithSDKConfigurationForInternalTelemetry`
+  feature gates. This change updates `metric_readers` to `readers` to align with the configuration
+  working group.

extension/zpagesextension/go.mod

@@ -27,6 +27,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.1-0.20220423185008-bf980b35cac4 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/collector/config/configtelemetry v0.81.0 // indirect
 	go.opentelemetry.io/collector/featuregate v1.0.0-rcv0013 // indirect
 	go.opentelemetry.io/collector/pdata v1.0.0-rcv0013 // indirect

extension/zpagesextension/go.sum

@@ -66,6 +66,8 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
+github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
+github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE=
 github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
@@ -90,6 +92,7 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -233,11 +236,16 @@ github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrf
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
@@ -246,6 +254,8 @@ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1
 go.etcd.io/etcd/api/v3 v3.5.4/go.mod h1:5GB2vv4A4AOn3yk7MftYGHkUfGtDHnEraIjym4dYz5A=
 go.etcd.io/etcd/client/pkg/v3 v3.5.4/go.mod h1:IJHfcCEKxYu1Os13ZdwCwIUTUVGYTSAM3YSwc9/Ac1g=
 go.etcd.io/etcd/client/v3 v3.5.4/go.mod h1:ZaRkVgBZC+L+dLCjTcF1hRXpgZXQPOvnA/Ak/gq3kiY=
+go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
+go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
 go.opentelemetry.io/contrib/zpages v0.42.0 h1:hFscXKQ9PTjyIVmAr6zIV8cMoiEeR9lPIwPVqHi8+5Q=
 go.opentelemetry.io/contrib/zpages v0.42.0/go.mod h1:qRJBEfB0iwRKrYImq5qfwTolmY8HXvZBRucvhuTVQZw=
 go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s=
@@ -294,6 +304,7 @@ golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLL
 golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
+golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
 golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1/go.mod h1:9tjilg8BloeKEkVJvy7fQ90B1CfIiPueXVOjqfkSzI8=
@@ -385,6 +396,7 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
 google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
 google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0=
+google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc=
 google.golang.org/grpc v1.38.0/go.mod h1:NREThFqKR1f3iQ6oBuvc5LadQuXVGo9rkm5ZGrQdJfM=
 google.golang.org/grpc v1.56.2 h1:fVRFRnXvU+x6C4IlHZewvJOVHoOv1TUuQyoRsYnB4bI=
 google.golang.org/grpc v1.56.2/go.mod h1:I9bI3vqKfayGqPUAwGdOSu7kt6oIJLixfffKrpXqQ9s=

go.mod

@@ -5,7 +5,6 @@ go 1.19
 require (
 	contrib.go.opencensus.io/exporter/prometheus v0.4.2
 	github.com/google/uuid v1.3.0
-	github.com/mitchellh/mapstructure v1.5.1-0.20220423185008-bf980b35cac4
 	github.com/prometheus/client_golang v1.16.0
 	github.com/prometheus/client_model v0.4.0
 	github.com/prometheus/common v0.44.0
@@ -62,6 +61,7 @@ require (
 	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
+	github.com/mitchellh/mapstructure v1.5.1-0.20220423185008-bf980b35cac4 // indirect
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect

service/internal/proctelemetry/config.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"strings"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -32,12 +31,6 @@ const (
 
 	// http Instrumentation Name
 	HTTPInstrumentation = "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
-
-	// supported metric readers
-	pullMetricReader = "pull"
-
-	// supported exporters
-	prometheusExporter = "prometheus"
 )
 
 var (
@@ -55,18 +48,11 @@ var (
 	}
 )
 
-func InitMetricReader(ctx context.Context, name string, reader any, asyncErrorChannel chan error) (sdkmetric.Reader, *http.Server, error) {
-	readerType := strings.Split(name, "/")[0]
-	switch readerType {
-	case pullMetricReader:
-		r, ok := reader.(telemetry.PullMetricReader)
-		if !ok {
-			return nil, nil, fmt.Errorf("invalid metric reader configuration: %v", reader)
-		}
-		return initExporter(ctx, r.Exporter, asyncErrorChannel)
-	default:
-		return nil, nil, fmt.Errorf("unsupported metric reader type %q", readerType)
+func InitMetricReader(ctx context.Context, reader telemetry.MetricReader, asyncErrorChannel chan error) (sdkmetric.Reader, *http.Server, error) {
+	if reader.Pull != nil {
+		return initExporter(ctx, reader.Pull.Exporter, asyncErrorChannel)
 	}
+	return nil, nil, fmt.Errorf("unsupported metric reader type %v", reader)
 }
 
 func InitOpenTelemetry(res *resource.Resource, options []sdkmetric.Option, disableHighCardinality bool) (*sdkmetric.MeterProvider, error) {
@@ -148,11 +134,9 @@ func initPrometheusExporter(prometheusConfig *telemetry.Prometheus, asyncErrorCh
 		return nil, nil, fmt.Errorf("port must be specified")
 	}
 	wrappedRegisterer := prometheus.WrapRegistererWithPrefix("otelcol_", promRegistry)
-	// We can remove `otelprom.WithoutUnits()` when the otel-go start exposing prometheus metrics using the OpenMetrics format
-	// which includes metric units that prometheusreceiver uses to trim unit's suffixes from metric names.
-	// https://github.com/open-telemetry/opentelemetry-go/issues/3468
 	exporter, err := otelprom.New(
 		otelprom.WithRegisterer(wrappedRegisterer),
+		// https://github.com/open-telemetry/opentelemetry-collector/issues/8043
 		otelprom.WithoutUnits(),
 		// Disabled for the moment until this becomes stable, and we are ready to break backwards compatibility.
 		otelprom.WithoutScopeInfo())

service/internal/proctelemetry/config_test.go

@@ -24,50 +24,56 @@ func intPtr(i int) *int {
 func TestMetricReader(t *testing.T) {
 	testCases := []struct {
 		name   string
-		reader any
+		reader telemetry.MetricReader
 		args   any
 		err    error
 	}{
 		{
 			name: "noreader",
-			err:  errors.New("unsupported metric reader type \"noreader\""),
+			err:  errors.New("unsupported metric reader type {<nil> <nil>}"),
 		},
 		{
 			name: "pull/prometheus-invalid-config-no-host",
-			reader: telemetry.PullMetricReader{
-				Exporter: telemetry.MetricExporter{
-					Prometheus: &telemetry.Prometheus{},
+			reader: telemetry.MetricReader{
+				Pull: &telemetry.PullMetricReader{
+					Exporter: telemetry.MetricExporter{
+						Prometheus: &telemetry.Prometheus{},
+					},
 				},
 			},
 			err: errors.New("host must be specified"),
 		},
 		{
 			name: "pull/prometheus-invalid-config-no-port",
-			reader: telemetry.PullMetricReader{
-				Exporter: telemetry.MetricExporter{
-					Prometheus: &telemetry.Prometheus{
-						Host: strPtr("locahost"),
+			reader: telemetry.MetricReader{
+				Pull: &telemetry.PullMetricReader{
+					Exporter: telemetry.MetricExporter{
+						Prometheus: &telemetry.Prometheus{
+							Host: strPtr("locahost"),
+						},
 					},
 				},
 			},
 			err: errors.New("port must be specified"),
 		},
 		{
 			name: "pull/prometheus-invalid-config-no-port",
-			reader: telemetry.PullMetricReader{
-				Exporter: telemetry.MetricExporter{
-					Prometheus: &telemetry.Prometheus{
-						Host: strPtr("locahost"),
-						Port: intPtr(8080),
+			reader: telemetry.MetricReader{
+				Pull: &telemetry.PullMetricReader{
+					Exporter: telemetry.MetricExporter{
+						Prometheus: &telemetry.Prometheus{
+							Host: strPtr("locahost"),
+							Port: intPtr(8080),
+						},
 					},
 				},
 			},
 		},
 	}
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			_, _, err := InitMetricReader(context.Background(), tc.name, tc.reader, make(chan error))
-			assert.Equal(t, tc.err, err)
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			_, _, err := InitMetricReader(context.Background(), tt.reader, make(chan error))
+			assert.Equal(t, tt.err, err)
 		})
 	}
 }

service/telemetry.go

@@ -108,26 +108,25 @@ func (tel *telemetryInitializer) initMetrics(res *resource.Resource, logger *zap
 			return err
 		}
 		if cfg.Metrics.Readers == nil {
-			cfg.Metrics.Readers = telemetry.MeterProviderJsonReaders{}
+			cfg.Metrics.Readers = []telemetry.MetricReader{}
 		}
-		cfg.Metrics.Readers["pull/serviceaddress"] = telemetry.PullMetricReader{
-			Exporter: telemetry.MetricExporter{
-				Prometheus: &telemetry.Prometheus{
-					Host: &host,
-					Port: &portInt,
+		cfg.Metrics.Readers = append(cfg.Metrics.Readers, telemetry.MetricReader{
+			Pull: &telemetry.PullMetricReader{
+				Exporter: telemetry.MetricExporter{
+					Prometheus: &telemetry.Prometheus{
+						Host: &host,
+						Port: &portInt,
+					},
 				},
 			},
-		}
+		})
 	}
 
 	metricproducer.GlobalManager().AddProducer(tel.ocRegistry)
 	opts := []sdkmetric.Option{}
-	for name, reader := range cfg.Metrics.Readers {
-		// @codeboten: server returned here only happens when a pull based metric
-		// reader is configured, this could be refactored to pass in a
-		// func to add the server to the list of servers. another thing that would
-		// be nice is not to have to pass down the asyncErrorChannel
-		r, server, err := proctelemetry.InitMetricReader(context.Background(), name, reader, asyncErrorChannel)
+	for _, reader := range cfg.Metrics.Readers {
+		// https://github.com/open-telemetry/opentelemetry-collector/issues/8045
+		r, server, err := proctelemetry.InitMetricReader(context.Background(), reader, asyncErrorChannel)
 		if err != nil {
 			return err
 		}

service/telemetry/config.go

@@ -5,13 +5,12 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
 	"fmt"
-	"strings"
 
-	"github.com/mitchellh/mapstructure"
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/config/configtelemetry"
 	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/internal/obsreportconfig"
 )
 
 // Config defines the configurable settings for service telemetry.
@@ -111,7 +110,7 @@ type MetricsConfig struct {
 
 	// Readers allow configuration of metric readers to emit metrics to
 	// any number of supported backends.
-	Readers MeterProviderJsonReaders `mapstructure:"readers"`
+	Readers []MetricReader `mapstructure:"readers"`
 }
 
 // TracesConfig exposes the common Telemetry configuration for collector's internal spans.
@@ -127,37 +126,33 @@ type TracesConfig struct {
 func (c *Config) Validate() error {
 	// Check when service telemetry metric level is not none, the metrics address should not be empty
 	if c.Metrics.Level != configtelemetry.LevelNone && c.Metrics.Address == "" && len(c.Metrics.Readers) == 0 {
-		return fmt.Errorf("collector telemetry metric address should exist when metric level is not none")
+		return fmt.Errorf("collector telemetry metric address or reader should exist when metric level is not none")
 	}
 
 	return nil
 }
 
-func (mrs *MeterProviderJsonReaders) Unmarshal(conf *confmap.Conf) error {
+func (mr *MetricReader) Unmarshal(conf *confmap.Conf) error {
+	if !obsreportconfig.UseOtelWithSDKConfigurationForInternalTelemetryFeatureGate.IsEnabled() {
+		fmt.Println(obsreportconfig.UseOtelWithSDKConfigurationForInternalTelemetryFeatureGate.IsEnabled())
+		// only unmarshal if feature gate is enabled
+		return nil
+	}
+
 	if conf == nil {
 		return nil
 	}
 
-	if err := conf.Unmarshal(mrs); err != nil {
-		return err
+	if err := conf.Unmarshal(mr); err != nil {
+		return fmt.Errorf("invalid metric reader configuration: %w", err)
 	}
 
-	for key, reader := range *mrs {
-		readerType := strings.Split(key, "/")[0]
-		switch readerType {
-		case "pull":
-			var r PullMetricReader
-			if err := mapstructure.Decode(reader, &r); err != nil {
-				return fmt.Errorf("invalid pull metric reader configuration: %w", err)
-			}
-			(*mrs)[key] = r
-			if r.Exporter.Prometheus == nil {
-				return fmt.Errorf("invalid exporter configuration")
-			}
-
-		default:
-			return fmt.Errorf("unsupported metric reader type %q", readerType)
+	if mr.Pull != nil {
+		if mr.Pull.Exporter.Prometheus == nil {
+			return fmt.Errorf("invalid exporter configuration")
 		}
+		return nil
 	}
-	return nil
+
+	return fmt.Errorf("unsupported metric reader type %s", conf.AllKeys())
 }