Skip to content

Commit

Permalink
adding test for extendedConfig w/ metric readers
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Boten <aboten@lightstep.com>
  • Loading branch information
Alex Boten committed Jun 22, 2023
1 parent 8b4d93b commit 79d744c
Show file tree
Hide file tree
Showing 13 changed files with 389 additions and 82 deletions.
21 changes: 21 additions & 0 deletions .chloggen/codeboten_may-29-otlp-export.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# 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 `metric_readers`"

# One or more tracking issues or pull requests related to the change
issues: [7641]

# (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 `metric_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.
1 change: 1 addition & 0 deletions exporter/otlpexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ require (
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.42.1-0.20230612162650-64be7e574a17 // indirect
go.opentelemetry.io/otel v1.16.0 // indirect
go.opentelemetry.io/otel/metric v1.16.0 // indirect
go.opentelemetry.io/otel/sdk v1.16.0 // indirect
go.opentelemetry.io/otel/trace v1.16.0 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions exporter/otlpexporter/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ go.opentelemetry.io/otel/exporters/prometheus v0.39.0 h1:whAaiHxOatgtKd+w0dOi//1
go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo=
go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxxNYodqc4xnGCo4=
go.opentelemetry.io/otel/sdk v1.16.0 h1:Z1Ok1YsijYL0CSJpHt4cS3wDDh7p572grzNrBMiMWgE=
go.opentelemetry.io/otel/sdk v1.16.0/go.mod h1:tMsIuKXuuIWPBAOrH+eHtvhTL+SntFtXF9QD68aP6p4=
go.opentelemetry.io/otel/sdk/metric v0.39.0 h1:Kun8i1eYf48kHH83RucG93ffz0zGV1sh46FAScOTuDI=
go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs=
go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0=
Expand Down
1 change: 1 addition & 0 deletions exporter/otlphttpexporter/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ require (
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 // indirect
go.opentelemetry.io/otel v1.16.0 // indirect
go.opentelemetry.io/otel/metric v1.16.0 // indirect
go.opentelemetry.io/otel/sdk v1.16.0 // indirect
go.opentelemetry.io/otel/trace v1.16.0 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
Expand Down
1 change: 1 addition & 0 deletions exporter/otlphttpexporter/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,7 @@ go.opentelemetry.io/otel/exporters/prometheus v0.39.0 h1:whAaiHxOatgtKd+w0dOi//1
go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo=
go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxxNYodqc4xnGCo4=
go.opentelemetry.io/otel/sdk v1.16.0 h1:Z1Ok1YsijYL0CSJpHt4cS3wDDh7p572grzNrBMiMWgE=
go.opentelemetry.io/otel/sdk v1.16.0/go.mod h1:tMsIuKXuuIWPBAOrH+eHtvhTL+SntFtXF9QD68aP6p4=
go.opentelemetry.io/otel/sdk/metric v0.39.0 h1:Kun8i1eYf48kHH83RucG93ffz0zGV1sh46FAScOTuDI=
go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs=
go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ 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.0
github.com/prometheus/client_golang v1.16.0
github.com/prometheus/client_model v0.4.0
github.com/prometheus/common v0.44.0
Expand Down Expand Up @@ -60,7 +61,6 @@ 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.0 // 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
Expand Down
1 change: 1 addition & 0 deletions internal/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/service/telemetry"
)

Expand Down
87 changes: 87 additions & 0 deletions service/internal/proctelemetry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,25 @@
package proctelemetry // import "go.opentelemetry.io/collector/service/internal/proctelemetry"

import (
"context"
"errors"
"fmt"
"net/http"
"strings"

"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/bridge/opencensus"
otelprom "go.opentelemetry.io/otel/exporters/prometheus"
"go.opentelemetry.io/otel/sdk/instrumentation"
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/resource"

"go.opentelemetry.io/collector/obsreport"
semconv "go.opentelemetry.io/collector/semconv/v1.18.0"
"go.opentelemetry.io/collector/service/telemetry"
)

const (
Expand All @@ -21,6 +32,12 @@ const (

// http Instrumentation Name
HTTPInstrumentation = "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"

// supported metric readers
pullMetricReader = "pull"

// supported exporters
prometheueExporter = "prometheus"
)

var (
Expand All @@ -38,6 +55,20 @@ 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)
}

Check warning on line 65 in service/internal/proctelemetry/config.go

View check run for this annotation

Codecov / codecov/patch

service/internal/proctelemetry/config.go#L64-L65

Added lines #L64 - L65 were not covered by tests
return initExporter(ctx, r.Exporter, asyncErrorChannel)
default:
return nil, nil, fmt.Errorf("unsupported metric reader type: %s", readerType)
}
}

func InitOpenTelemetry(res *resource.Resource, options []sdkmetric.Option, disableHighCardinality bool) (*sdkmetric.MeterProvider, error) {
opts := []sdkmetric.Option{
sdkmetric.WithResource(res),
Expand All @@ -50,6 +81,21 @@ func InitOpenTelemetry(res *resource.Resource, options []sdkmetric.Option, disab
), nil
}

func InitPrometheusServer(registry *prometheus.Registry, address string, asyncErrorChannel chan error) *http.Server {
mux := http.NewServeMux()
mux.Handle("/metrics", promhttp.HandlerFor(registry, promhttp.HandlerOpts{}))
server := &http.Server{
Addr: address,
Handler: mux,
}
go func() {
if serveErr := server.ListenAndServe(); serveErr != nil && !errors.Is(serveErr, http.ErrServerClosed) {
asyncErrorChannel <- serveErr
}
}()
return server
}

func batchViews(disableHighCardinality bool) []sdkmetric.View {
views := []sdkmetric.View{
sdkmetric.NewView(
Expand Down Expand Up @@ -92,3 +138,44 @@ func cardinalityFilter(kvs ...attribute.KeyValue) attribute.Filter {
return !filter.HasValue(kv.Key)
}
}

func initPrometheusExporter(prometheusConfig telemetry.Prometheus, asyncErrorChannel chan error) (sdkmetric.Reader, *http.Server, error) {
promRegistry := prometheus.NewRegistry()
if prometheusConfig.Host == nil {
return nil, nil, fmt.Errorf("host must be specified")
}
if prometheusConfig.Port == nil {
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),
otelprom.WithoutUnits(),
// Disabled for the moment until this becomes stable, and we are ready to break backwards compatibility.
otelprom.WithoutScopeInfo())
if err != nil {
return nil, nil, fmt.Errorf("error creating otel prometheus exporter: %w", err)
}

Check warning on line 161 in service/internal/proctelemetry/config.go

View check run for this annotation

Codecov / codecov/patch

service/internal/proctelemetry/config.go#L160-L161

Added lines #L160 - L161 were not covered by tests

exporter.RegisterProducer(opencensus.NewMetricProducer())
return exporter, InitPrometheusServer(promRegistry, fmt.Sprintf("%s:%d", *prometheusConfig.Host, *prometheusConfig.Port), asyncErrorChannel), nil
}

func initExporter(_ context.Context, exporters telemetry.MetricExporter, asyncErrorChannel chan error) (sdkmetric.Reader, *http.Server, error) {
for exporterType, exporter := range exporters {
switch exporterType {
case prometheueExporter:
e, ok := exporter.(telemetry.Prometheus)
if !ok {
return nil, nil, fmt.Errorf("prometheus exporter invalid: %v", exporter)
}

Check warning on line 174 in service/internal/proctelemetry/config.go

View check run for this annotation

Codecov / codecov/patch

service/internal/proctelemetry/config.go#L173-L174

Added lines #L173 - L174 were not covered by tests
return initPrometheusExporter(e, asyncErrorChannel)
default:
return nil, nil, fmt.Errorf("unsupported metric exporter type: %s", exporterType)

Check warning on line 177 in service/internal/proctelemetry/config.go

View check run for this annotation

Codecov / codecov/patch

service/internal/proctelemetry/config.go#L176-L177

Added lines #L176 - L177 were not covered by tests
}
}
return nil, nil, fmt.Errorf("no valid exporter: %v", exporters)

Check warning on line 180 in service/internal/proctelemetry/config.go

View check run for this annotation

Codecov / codecov/patch

service/internal/proctelemetry/config.go#L180

Added line #L180 was not covered by tests
}
69 changes: 69 additions & 0 deletions service/internal/proctelemetry/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,72 @@
// SPDX-License-Identifier: Apache-2.0

package proctelemetry

import (
"context"
"errors"
"testing"

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/service/telemetry"
)

func strPtr(s string) *string {
return &s
}

func intPtr(i int) *int {
return &i
}

func TestMetricReader(t *testing.T) {
testCases := []struct {
name string
reader any
args any
err error
}{
{
name: "noreader",
err: errors.New("unsupported metric reader type: noreader"),
},
{
name: "pull/prometheus-invalid-config-no-host",
reader: 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"),
},
},
},
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),
},
},
},
},
}
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)
})
}
}
Loading

0 comments on commit 79d744c

Please sign in to comment.