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())
 }