From 34e94fd6f988e6bcd2e62df4ee489fccb43a5f67 Mon Sep 17 00:00:00 2001 From: Moh Osman <59479562+moh-osman3@users.noreply.github.com> Date: Tue, 1 Nov 2022 14:13:11 -0400 Subject: [PATCH 01/11] Instrument obsreport.Scraper (#19) * instrument obsreport.scraper metrics with otel go --- internal/obsreportconfig/obsreportconfig.go | 21 ++-- obsreport/obsreport_scraper.go | 85 +++++++++++++--- obsreport/obsreport_test.go | 97 +++++++++---------- obsreport/obsreporttest/obsreporttest.go | 15 +-- obsreport/obsreporttest/obsreporttest_test.go | 25 ++++- .../obsreporttest/otelprometheuschecker.go | 15 ++- 6 files changed, 173 insertions(+), 85 deletions(-) diff --git a/internal/obsreportconfig/obsreportconfig.go b/internal/obsreportconfig/obsreportconfig.go index 6636e946460..483a93dabe5 100644 --- a/internal/obsreportconfig/obsreportconfig.go +++ b/internal/obsreportconfig/obsreportconfig.go @@ -71,12 +71,7 @@ func allViews() []*view.View { views = append(views, receiverViews()...) // Scraper views. - measures = []*stats.Int64Measure{ - obsmetrics.ScraperScrapedMetricPoints, - obsmetrics.ScraperErroredMetricPoints, - } - tagKeys = []tag.Key{obsmetrics.TagKeyReceiver, obsmetrics.TagKeyScraper} - views = append(views, genViews(measures, tagKeys, view.Sum())...) + views = append(views, scraperViews()...) // Exporter views. measures = []*stats.Int64Measure{ @@ -136,6 +131,20 @@ func receiverViews() []*view.View { return genViews(measures, tagKeys, view.Sum()) } +func scraperViews() []*view.View { + if featuregate.GetRegistry().IsEnabled(UseOtelForInternalMetricsfeatureGateID) { + return nil + } + + measures := []*stats.Int64Measure{ + obsmetrics.ScraperScrapedMetricPoints, + obsmetrics.ScraperErroredMetricPoints, + } + tagKeys := []tag.Key{obsmetrics.TagKeyReceiver, obsmetrics.TagKeyScraper} + + return genViews(measures, tagKeys, view.Sum()) +} + func genViews( measures []*stats.Int64Measure, tagKeys []tag.Key, diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index d054ea5cf5d..e8438e00065 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -21,14 +21,25 @@ import ( "go.opencensus.io/stats" "go.opencensus.io/tag" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/instrument" + "go.opentelemetry.io/otel/metric/instrument/syncint64" + "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configtelemetry" + "go.opentelemetry.io/collector/featuregate" + "go.opentelemetry.io/collector/internal/obsreportconfig" "go.opentelemetry.io/collector/internal/obsreportconfig/obsmetrics" "go.opentelemetry.io/collector/receiver/scrapererror" ) +var ( + scraperName = "scraper" + scraperScope = scopeName + nameSep + scraperName +) + // Scraper is a helper to add observability to a component.Scraper. type Scraper struct { level configtelemetry.Level @@ -36,6 +47,13 @@ type Scraper struct { scraper component.ID mutators []tag.Mutator tracer trace.Tracer + + logger *zap.Logger + + useOtelForMetrics bool + otelAttrs []attribute.KeyValue + scrapedMetricsPoints syncint64.Counter + erroredMetricsPoints syncint64.Counter } // ScraperSettings are settings for creating a Scraper. @@ -46,8 +64,12 @@ type ScraperSettings struct { } // NewScraper creates a new Scraper. -func NewScraper(cfg ScraperSettings) (*Scraper, error) { - return &Scraper{ +func NewScraper(cfg ScraperSettings) *Scraper { + return newScraper(cfg, featuregate.GetRegistry()) +} + +func newScraper(cfg ScraperSettings, registry *featuregate.Registry) *Scraper { + scraper := &Scraper{ level: cfg.ReceiverCreateSettings.TelemetrySettings.MetricsLevel, receiverID: cfg.ReceiverID, scraper: cfg.Scraper, @@ -55,17 +77,45 @@ func NewScraper(cfg ScraperSettings) (*Scraper, error) { tag.Upsert(obsmetrics.TagKeyReceiver, cfg.ReceiverID.String(), tag.WithTTL(tag.TTLNoPropagation)), tag.Upsert(obsmetrics.TagKeyScraper, cfg.Scraper.String(), tag.WithTTL(tag.TTLNoPropagation))}, tracer: cfg.ReceiverCreateSettings.TracerProvider.Tracer(cfg.Scraper.String()), - }, nil + + logger: cfg.ReceiverCreateSettings.Logger, + useOtelForMetrics: registry.IsEnabled(obsreportconfig.UseOtelForInternalMetricsfeatureGateID), + otelAttrs: []attribute.KeyValue{ + attribute.String(obsmetrics.ReceiverKey, cfg.ReceiverID.String()), + attribute.String(obsmetrics.ScraperKey, cfg.Scraper.String()), + }, + } + + scraper.createOtelMetrics(cfg) + return scraper } -// Deprecated: [v0.65.0] use NewScraper. -func MustNewScraper(cfg ScraperSettings) *Scraper { - scrap, err := NewScraper(cfg) - if err != nil { - panic(err) +func (s *Scraper) createOtelMetrics(cfg ScraperSettings) { + if !s.useOtelForMetrics { + return } + meter := cfg.ReceiverCreateSettings.MeterProvider.Meter(scraperScope) - return scrap + var err error + handleError := func(metricName string, err error) { + if err != nil { + s.logger.Warn("failed to create otel instrument", zap.Error(err), zap.String("metric", metricName)) + } + } + + s.scrapedMetricsPoints, err = meter.SyncInt64().Counter( + obsmetrics.ScraperPrefix+obsmetrics.ScrapedMetricPointsKey, + instrument.WithDescription("Number of metric points successfully scraped."), + instrument.WithUnit(unit.Dimensionless), + ) + handleError(obsmetrics.ScraperPrefix+obsmetrics.ScrapedMetricPointsKey, err) + + s.erroredMetricsPoints, err = meter.SyncInt64().Counter( + obsmetrics.ScraperPrefix+obsmetrics.ErroredMetricPointsKey, + instrument.WithDescription("Number of metric points that were unable to be scraped."), + instrument.WithUnit(unit.Dimensionless), + ) + handleError(obsmetrics.ScraperPrefix+obsmetrics.ErroredMetricPointsKey, err) } // StartMetricsOp is called when a scrape operation is started. The @@ -100,10 +150,7 @@ func (s *Scraper) EndMetricsOp( span := trace.SpanFromContext(scraperCtx) if s.level != configtelemetry.LevelNone { - stats.Record( - scraperCtx, - obsmetrics.ScraperScrapedMetricPoints.M(int64(numScrapedMetrics)), - obsmetrics.ScraperErroredMetricPoints.M(int64(numErroredMetrics))) + s.recordMetrics(scraperCtx, numScrapedMetrics, numErroredMetrics) } // end span according to errors @@ -118,3 +165,15 @@ func (s *Scraper) EndMetricsOp( span.End() } + +func (s *Scraper) recordMetrics(scraperCtx context.Context, numScrapedMetrics, numErroredMetrics int) { + if s.useOtelForMetrics { + s.scrapedMetricsPoints.Add(scraperCtx, int64(numScrapedMetrics), s.otelAttrs...) + s.erroredMetricsPoints.Add(scraperCtx, int64(numErroredMetrics), s.otelAttrs...) + } else { // OC for metrics + stats.Record( + scraperCtx, + obsmetrics.ScraperScrapedMetricPoints.M(int64(numScrapedMetrics)), + obsmetrics.ScraperErroredMetricPoints.M(int64(numErroredMetrics))) + } +} \ No newline at end of file diff --git a/obsreport/obsreport_test.go b/obsreport/obsreport_test.go index 4c8179cdaca..445f430219b 100644 --- a/obsreport/obsreport_test.go +++ b/obsreport/obsreport_test.go @@ -219,62 +219,59 @@ func TestReceiveMetricsOp(t *testing.T) { } func TestScrapeMetricsDataOp(t *testing.T) { - tt, err := obsreporttest.SetupTelemetry() - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - - parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) - defer parentSpan.End() + testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) + defer parentSpan.End() - params := []testParams{ - {items: 23, err: partialErrFake}, - {items: 29, err: errFake}, - {items: 15, err: nil}, - } - for i := range params { - scrp, serr := NewScraper(ScraperSettings{ - ReceiverID: receiver, - Scraper: scraper, - ReceiverCreateSettings: tt.ToReceiverCreateSettings(), - }) - require.NoError(t, serr) - ctx := scrp.StartMetricsOp(parentCtx) - assert.NotNil(t, ctx) - scrp.EndMetricsOp(ctx, params[i].items, params[i].err) - } + params := []testParams{ + {items: 23, err: partialErrFake}, + {items: 29, err: errFake}, + {items: 15, err: nil}, + } + for i := range params { + scrp := newScraper(ScraperSettings{ + ReceiverID: receiver, + Scraper: scraper, + ReceiverCreateSettings: tt.ToReceiverCreateSettings(), + }, registry) + ctx := scrp.StartMetricsOp(parentCtx) + assert.NotNil(t, ctx) + scrp.EndMetricsOp(ctx, params[i].items, params[i].err) + } - spans := tt.SpanRecorder.Ended() - require.Equal(t, len(params), len(spans)) + spans := tt.SpanRecorder.Ended() + require.Equal(t, len(params), len(spans)) - var scrapedMetricPoints, erroredMetricPoints int - for i, span := range spans { - assert.Equal(t, "scraper/"+receiver.String()+"/"+scraper.String()+"/MetricsScraped", span.Name()) - switch { - case params[i].err == nil: - scrapedMetricPoints += params[i].items - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(0)}) - assert.Equal(t, codes.Unset, span.Status().Code) - case errors.Is(params[i].err, errFake): - erroredMetricPoints += params[i].items - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(0)}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - assert.Equal(t, codes.Error, span.Status().Code) - assert.Equal(t, params[i].err.Error(), span.Status().Description) + var scrapedMetricPoints, erroredMetricPoints int + for i, span := range spans { + assert.Equal(t, "scraper/"+receiver.String()+"/"+scraper.String()+"/MetricsScraped", span.Name()) + switch { + case params[i].err == nil: + scrapedMetricPoints += params[i].items + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(0)}) + assert.Equal(t, codes.Unset, span.Status().Code) + case errors.Is(params[i].err, errFake): + erroredMetricPoints += params[i].items + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(0)}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + assert.Equal(t, codes.Error, span.Status().Code) + assert.Equal(t, params[i].err.Error(), span.Status().Description) - case errors.Is(params[i].err, partialErrFake): - scrapedMetricPoints += params[i].items - erroredMetricPoints++ - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(1)}) - assert.Equal(t, codes.Error, span.Status().Code) - assert.Equal(t, params[i].err.Error(), span.Status().Description) - default: - t.Fatalf("unexpected err param: %v", params[i].err) + case errors.Is(params[i].err, partialErrFake): + scrapedMetricPoints += params[i].items + erroredMetricPoints++ + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(1)}) + assert.Equal(t, codes.Error, span.Status().Code) + assert.Equal(t, params[i].err.Error(), span.Status().Description) + default: + t.Fatalf("unexpected err param: %v", params[i].err) + } } - } - require.NoError(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, int64(scrapedMetricPoints), int64(erroredMetricPoints))) + require.NoError(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, int64(scrapedMetricPoints), int64(erroredMetricPoints))) + }) } func TestExportTraceDataOp(t *testing.T) { diff --git a/obsreport/obsreporttest/obsreporttest.go b/obsreport/obsreporttest/obsreporttest.go index 4fcd7185673..76451bac84b 100644 --- a/obsreport/obsreporttest/obsreporttest.go +++ b/obsreport/obsreporttest/obsreporttest.go @@ -205,11 +205,8 @@ func CheckReceiverMetrics(tts TestTelemetry, receiver component.ID, protocol str // CheckScraperMetrics checks that for the current exported values for metrics scraper metrics match given values. // When this function is called it is required to also call SetupTelemetry as first thing. -func CheckScraperMetrics(_ TestTelemetry, receiver component.ID, scraper component.ID, scrapedMetricPoints, erroredMetricPoints int64) error { - scraperTags := tagsForScraperView(receiver, scraper) - return multierr.Combine( - checkValueForView(scraperTags, scrapedMetricPoints, "scraper/scraped_metric_points"), - checkValueForView(scraperTags, erroredMetricPoints, "scraper/errored_metric_points")) +func CheckScraperMetrics(tts TestTelemetry, receiver config.ComponentID, scraper config.ComponentID, scrapedMetricPoints, erroredMetricPoints int64) error { + return tts.otelPrometheusChecker.checkScraperMetrics(receiver, scraper, scrapedMetricPoints, erroredMetricPoints) } // checkValueForView checks that for the current exported value in the view with the given name @@ -237,14 +234,6 @@ func checkValueForView(wantTags []tag.Tag, value int64, vName string) error { return fmt.Errorf("[%s]: could not find tags, wantTags: %s in rows %v", vName, wantTags, rows) } -// tagsForScraperView returns the tags that are needed for the scraper views. -func tagsForScraperView(receiver component.ID, scraper component.ID) []tag.Tag { - return []tag.Tag{ - {Key: receiverTag, Value: receiver.String()}, - {Key: scraperTag, Value: scraper.String()}, - } -} - // tagsForProcessorView returns the tags that are needed for the processor views. func tagsForProcessorView(processor component.ID) []tag.Tag { return []tag.Tag{ diff --git a/obsreport/obsreporttest/obsreporttest_test.go b/obsreport/obsreporttest/obsreporttest_test.go index cdb42bd6e33..27d7dbd88ac 100644 --- a/obsreport/obsreporttest/obsreporttest_test.go +++ b/obsreport/obsreporttest/obsreporttest_test.go @@ -32,10 +32,31 @@ const ( ) var ( - receiver = component.NewID("fakeReicever") - exporter = component.NewID("fakeExporter") + scraper = config.NewComponentID("fakeScraper") + receiver = config.NewComponentID("fakeReicever") + exporter = config.NewComponentID("fakeExporter") ) +func TestCheckScraperMetricsViews(t *testing.T) { + tt, err := obsreporttest.SetupTelemetry() + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) + + s := obsreport.NewScraper(obsreport.ScraperSettings{ + ReceiverID: receiver, + Scraper: scraper, + ReceiverCreateSettings: tt.ToReceiverCreateSettings(), + }) + ctx := s.StartMetricsOp(context.Background()) + require.NotNil(t, ctx) + s.EndMetricsOp(ctx, 7, nil) + + assert.NoError(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, 7, 0)) + assert.Error(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, 7, 7)) + assert.Error(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, 0, 0)) + assert.Error(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, 0, 7)) +} + func TestCheckReceiverTracesViews(t *testing.T) { tt, err := obsreporttest.SetupTelemetry() require.NoError(t, err) diff --git a/obsreport/obsreporttest/otelprometheuschecker.go b/obsreport/obsreporttest/otelprometheuschecker.go index 3c0a4cad4cc..f1f0d19a4c8 100644 --- a/obsreport/obsreporttest/otelprometheuschecker.go +++ b/obsreport/obsreporttest/otelprometheuschecker.go @@ -34,7 +34,14 @@ type prometheusChecker struct { promHandler http.Handler } -func (pc *prometheusChecker) checkReceiverTraces(receiver component.ID, protocol string, acceptedSpans, droppedSpans int64) error { +func (pc *prometheusChecker) checkScraperMetrics(receiver config.ComponentID, scraper config.ComponentID, scrapedMetricPoints, erroredMetricPoints int64) error{ + scraperAttrs := attributesForScraperMetrics(receiver, scraper) + return multierr.Combine( + pc.checkCounter("scraper_scraped_metric_points", scrapedMetricPoints, scraperAttrs), + pc.checkCounter("scraper_errored_metric_points", erroredMetricPoints, scraperAttrs)) +} + +func (pc *prometheusChecker) checkReceiverTraces(receiver config.ComponentID, protocol string, acceptedSpans, droppedSpans int64) error { receiverAttrs := attributesForReceiverMetrics(receiver, protocol) return multierr.Combine( pc.checkCounter("receiver_accepted_spans", acceptedSpans, receiverAttrs), @@ -145,6 +152,12 @@ func fetchPrometheusMetrics(handler http.Handler) (map[string]*io_prometheus_cli return parser.TextToMetricFamilies(rr.Body) } +func attributesForScraperMetrics(receiver config.ComponentID, scraper config.ComponentID) []attribute.KeyValue { + return []attribute.KeyValue{ + attribute.String(receiverTag.Name(), receiver.String()), + attribute.String(scraperTag.Name(), scraper.String()), + } +} // attributesForReceiverMetrics returns the attributes that are needed for the receiver metrics. func attributesForReceiverMetrics(receiver component.ID, transport string) []attribute.KeyValue { return []attribute.KeyValue{ From 0f05e8b4d8a303ee3fcfd6d4c6edb5e72a2f215e Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Tue, 1 Nov 2022 22:46:52 -0400 Subject: [PATCH 02/11] add changelog --- .chloggen/obsreport-scraper.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .chloggen/obsreport-scraper.yaml diff --git a/.chloggen/obsreport-scraper.yaml b/.chloggen/obsreport-scraper.yaml new file mode 100644 index 00000000000..7a20549b68e --- /dev/null +++ b/.chloggen/obsreport-scraper.yaml @@ -0,0 +1,17 @@ + +# 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: obsreport + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Instrument `obsreport.Scraper` metrics with otel-go" + +# One or more tracking issues or pull requests related to the change +issues: [6460] + +# (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: \ No newline at end of file From 23001c98d8f78a02b93327a453a96342f37f59fe Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Thu, 3 Nov 2022 13:21:24 -0400 Subject: [PATCH 03/11] update API to use MustNewScraper --- obsreport/obsreport_scraper.go | 45 ++++++++++++------- obsreport/obsreport_test.go | 3 +- obsreport/obsreporttest/obsreporttest_test.go | 2 +- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index e8438e00065..0c8761860ee 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -25,6 +25,7 @@ import ( "go.opentelemetry.io/otel/metric/instrument/syncint64" "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/trace" + "go.uber.org/multierr" "go.uber.org/zap" "go.opentelemetry.io/collector/component" @@ -63,12 +64,26 @@ type ScraperSettings struct { ReceiverCreateSettings component.ReceiverCreateSettings } -// NewScraper creates a new Scraper. +// Deprecated: [v0.64.0] use MustNewScraper. func NewScraper(cfg ScraperSettings) *Scraper { - return newScraper(cfg, featuregate.GetRegistry()) + scr, err := newScraper(cfg, featuregate.GetRegistry()) + if err != nil && cfg.ReceiverCreateSettings.Logger != nil { + cfg.ReceiverCreateSettings.Logger.Warn("Error creating an obsreport.Scraper", zap.Error(err)) + } + return scr +} + +// MustNewReceiver creates a new Scraper. +func MustNewScraper(cfg ScraperSettings) *Scraper { + scr, err := newScraper(cfg, featuregate.GetRegistry()) + if err != nil { + panic(err) + } + + return scr } -func newScraper(cfg ScraperSettings, registry *featuregate.Registry) *Scraper { +func newScraper(cfg ScraperSettings, registry *featuregate.Registry) (*Scraper, error) { scraper := &Scraper{ level: cfg.ReceiverCreateSettings.TelemetrySettings.MetricsLevel, receiverID: cfg.ReceiverID, @@ -86,36 +101,36 @@ func newScraper(cfg ScraperSettings, registry *featuregate.Registry) *Scraper { }, } - scraper.createOtelMetrics(cfg) - return scraper + if err := scraper.createOtelMetrics(cfg); err != nil { + return nil, err + } + + return scraper, nil } -func (s *Scraper) createOtelMetrics(cfg ScraperSettings) { +func (s *Scraper) createOtelMetrics(cfg ScraperSettings) error { if !s.useOtelForMetrics { - return + return nil } meter := cfg.ReceiverCreateSettings.MeterProvider.Meter(scraperScope) - var err error - handleError := func(metricName string, err error) { - if err != nil { - s.logger.Warn("failed to create otel instrument", zap.Error(err), zap.String("metric", metricName)) - } - } + var errors, err error s.scrapedMetricsPoints, err = meter.SyncInt64().Counter( obsmetrics.ScraperPrefix+obsmetrics.ScrapedMetricPointsKey, instrument.WithDescription("Number of metric points successfully scraped."), instrument.WithUnit(unit.Dimensionless), ) - handleError(obsmetrics.ScraperPrefix+obsmetrics.ScrapedMetricPointsKey, err) + errors = multierr.Append(errors, err) s.erroredMetricsPoints, err = meter.SyncInt64().Counter( obsmetrics.ScraperPrefix+obsmetrics.ErroredMetricPointsKey, instrument.WithDescription("Number of metric points that were unable to be scraped."), instrument.WithUnit(unit.Dimensionless), ) - handleError(obsmetrics.ScraperPrefix+obsmetrics.ErroredMetricPointsKey, err) + errors = multierr.Append(errors, err) + + return errors } // StartMetricsOp is called when a scrape operation is started. The diff --git a/obsreport/obsreport_test.go b/obsreport/obsreport_test.go index 445f430219b..c1a71524597 100644 --- a/obsreport/obsreport_test.go +++ b/obsreport/obsreport_test.go @@ -229,11 +229,12 @@ func TestScrapeMetricsDataOp(t *testing.T) { {items: 15, err: nil}, } for i := range params { - scrp := newScraper(ScraperSettings{ + scrp, err := newScraper(ScraperSettings{ ReceiverID: receiver, Scraper: scraper, ReceiverCreateSettings: tt.ToReceiverCreateSettings(), }, registry) + require.NoError(t, err) ctx := scrp.StartMetricsOp(parentCtx) assert.NotNil(t, ctx) scrp.EndMetricsOp(ctx, params[i].items, params[i].err) diff --git a/obsreport/obsreporttest/obsreporttest_test.go b/obsreport/obsreporttest/obsreporttest_test.go index 27d7dbd88ac..e0c7270ef81 100644 --- a/obsreport/obsreporttest/obsreporttest_test.go +++ b/obsreport/obsreporttest/obsreporttest_test.go @@ -42,7 +42,7 @@ func TestCheckScraperMetricsViews(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - s := obsreport.NewScraper(obsreport.ScraperSettings{ + s := obsreport.MustNewScraper(obsreport.ScraperSettings{ ReceiverID: receiver, Scraper: scraper, ReceiverCreateSettings: tt.ToReceiverCreateSettings(), From 11c682cf69902a7193430ac26429e50b158f5793 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Thu, 3 Nov 2022 15:56:06 -0400 Subject: [PATCH 04/11] fix typo add testing for checkScraperMetrics --- obsreport/obsreport_scraper.go | 2 +- obsreport/obsreporttest/otelprometheuschecker_test.go | 11 +++++++++++ obsreport/obsreporttest/testdata/prometheus_response | 6 ++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index 0c8761860ee..6c1b16a0754 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -73,7 +73,7 @@ func NewScraper(cfg ScraperSettings) *Scraper { return scr } -// MustNewReceiver creates a new Scraper. +// MustNewScraper creates a new Scraper. func MustNewScraper(cfg ScraperSettings) *Scraper { scr, err := newScraper(cfg, featuregate.GetRegistry()) if err != nil { diff --git a/obsreport/obsreporttest/otelprometheuschecker_test.go b/obsreport/obsreporttest/otelprometheuschecker_test.go index cc8959fc711..15639ae6030 100644 --- a/obsreport/obsreporttest/otelprometheuschecker_test.go +++ b/obsreport/obsreporttest/otelprometheuschecker_test.go @@ -47,8 +47,14 @@ func TestPromChecker(t *testing.T) { pc, err := newStubPromChecker() require.NoError(t, err) +<<<<<<< HEAD receiver := component.NewID("fakeReceiver") exporter := component.NewID("fakeExporter") +======= + scraper := config.NewComponentID("fakeScraper") + receiver := config.NewComponentID("fakeReceiver") + exporter := config.NewComponentID("fakeExporter") +>>>>>>> bdc7135b (fix typo add testing for checkScraperMetrics) transport := "fakeTransport" assert.NoError(t, @@ -76,6 +82,11 @@ func TestPromChecker(t *testing.T) { "invalid metric type should return error", ) + assert.NoError(t, + pc.checkScraperMetrics(receiver, scraper, 7, 41), + "metrics from Scraper Metrics should be valid", + ) + assert.NoError(t, pc.checkReceiverTraces(receiver, transport, 42, 13), "metrics from Receiver Traces should be valid", diff --git a/obsreport/obsreporttest/testdata/prometheus_response b/obsreport/obsreporttest/testdata/prometheus_response index 0a0b4dc3c38..60680c8fb48 100644 --- a/obsreport/obsreporttest/testdata/prometheus_response +++ b/obsreport/obsreporttest/testdata/prometheus_response @@ -34,6 +34,12 @@ receiver_refused_metric_points{receiver="fakeReceiver",transport="fakeTransport" # HELP receiver_refused_spans Number of spans that could not be pushed into the pipeline. # TYPE receiver_refused_spans counter receiver_refused_spans{receiver="fakeReceiver",transport="fakeTransport"} 13 +# HELP scraper_scraped_metric_points Number of metric points successfully scraped. +# TYPE scraper_scraped_metric_points counter +scraper_scraped_metric_points{receiver="fakeReceiver",scraper="fakeScraper"} 7 +# HELP scraper_errored_metric_points Number of metric points that were unable to be scraped. +# TYPE scraper_errored_metric_points counter +scraper_errored_metric_points{receiver="fakeReceiver",scraper="fakeScraper"} 41 # HELP gauge_metric A simple gauge metric # TYPE gauge_metric gauge gauge_metric 49 From 238e1405d0876e8a82214b7ef2047dab5e909db7 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Wed, 9 Nov 2022 14:00:35 -0500 Subject: [PATCH 05/11] remove accidental merge conflict references --- obsreport/obsreporttest/otelprometheuschecker_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/obsreport/obsreporttest/otelprometheuschecker_test.go b/obsreport/obsreporttest/otelprometheuschecker_test.go index 15639ae6030..18dccd16269 100644 --- a/obsreport/obsreporttest/otelprometheuschecker_test.go +++ b/obsreport/obsreporttest/otelprometheuschecker_test.go @@ -47,14 +47,9 @@ func TestPromChecker(t *testing.T) { pc, err := newStubPromChecker() require.NoError(t, err) -<<<<<<< HEAD - receiver := component.NewID("fakeReceiver") - exporter := component.NewID("fakeExporter") -======= scraper := config.NewComponentID("fakeScraper") receiver := config.NewComponentID("fakeReceiver") exporter := config.NewComponentID("fakeExporter") ->>>>>>> bdc7135b (fix typo add testing for checkScraperMetrics) transport := "fakeTransport" assert.NoError(t, From e4dde754dd23ceb50705015fba6019c612eed652 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Mon, 14 Nov 2022 22:47:26 -0500 Subject: [PATCH 06/11] fix references after rebase --- .chloggen/obsreport-scraper.yaml | 2 +- obsreport/obsreport_scraper.go | 4 ++-- obsreport/obsreporttest/obsreporttest.go | 2 +- obsreport/obsreporttest/obsreporttest_test.go | 8 ++++---- obsreport/obsreporttest/otelprometheuschecker.go | 6 +++--- obsreport/obsreporttest/otelprometheuschecker_test.go | 6 +++--- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.chloggen/obsreport-scraper.yaml b/.chloggen/obsreport-scraper.yaml index 7a20549b68e..fcfa3545eba 100644 --- a/.chloggen/obsreport-scraper.yaml +++ b/.chloggen/obsreport-scraper.yaml @@ -14,4 +14,4 @@ issues: [6460] # (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: \ No newline at end of file +subtext: diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index 6c1b16a0754..12513a6637c 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -64,7 +64,7 @@ type ScraperSettings struct { ReceiverCreateSettings component.ReceiverCreateSettings } -// Deprecated: [v0.64.0] use MustNewScraper. +// NewScraper creates a new Scraper. func NewScraper(cfg ScraperSettings) *Scraper { scr, err := newScraper(cfg, featuregate.GetRegistry()) if err != nil && cfg.ReceiverCreateSettings.Logger != nil { @@ -73,7 +73,7 @@ func NewScraper(cfg ScraperSettings) *Scraper { return scr } -// MustNewScraper creates a new Scraper. +// Deprecated: [v0.65.0] use NewScraper. func MustNewScraper(cfg ScraperSettings) *Scraper { scr, err := newScraper(cfg, featuregate.GetRegistry()) if err != nil { diff --git a/obsreport/obsreporttest/obsreporttest.go b/obsreport/obsreporttest/obsreporttest.go index 76451bac84b..f9ed4634cfd 100644 --- a/obsreport/obsreporttest/obsreporttest.go +++ b/obsreport/obsreporttest/obsreporttest.go @@ -205,7 +205,7 @@ func CheckReceiverMetrics(tts TestTelemetry, receiver component.ID, protocol str // CheckScraperMetrics checks that for the current exported values for metrics scraper metrics match given values. // When this function is called it is required to also call SetupTelemetry as first thing. -func CheckScraperMetrics(tts TestTelemetry, receiver config.ComponentID, scraper config.ComponentID, scrapedMetricPoints, erroredMetricPoints int64) error { +func CheckScraperMetrics(tts TestTelemetry, receiver component.ID, scraper component.ID, scrapedMetricPoints, erroredMetricPoints int64) error { return tts.otelPrometheusChecker.checkScraperMetrics(receiver, scraper, scrapedMetricPoints, erroredMetricPoints) } diff --git a/obsreport/obsreporttest/obsreporttest_test.go b/obsreport/obsreporttest/obsreporttest_test.go index e0c7270ef81..03cfa629b1e 100644 --- a/obsreport/obsreporttest/obsreporttest_test.go +++ b/obsreport/obsreporttest/obsreporttest_test.go @@ -32,9 +32,9 @@ const ( ) var ( - scraper = config.NewComponentID("fakeScraper") - receiver = config.NewComponentID("fakeReicever") - exporter = config.NewComponentID("fakeExporter") + scraper = component.NewID("fakeScraper") + receiver = component.NewID("fakeReicever") + exporter = component.NewID("fakeExporter") ) func TestCheckScraperMetricsViews(t *testing.T) { @@ -42,7 +42,7 @@ func TestCheckScraperMetricsViews(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - s := obsreport.MustNewScraper(obsreport.ScraperSettings{ + s := obsreport.NewScraper(obsreport.ScraperSettings{ ReceiverID: receiver, Scraper: scraper, ReceiverCreateSettings: tt.ToReceiverCreateSettings(), diff --git a/obsreport/obsreporttest/otelprometheuschecker.go b/obsreport/obsreporttest/otelprometheuschecker.go index f1f0d19a4c8..327b0a9b18f 100644 --- a/obsreport/obsreporttest/otelprometheuschecker.go +++ b/obsreport/obsreporttest/otelprometheuschecker.go @@ -34,14 +34,14 @@ type prometheusChecker struct { promHandler http.Handler } -func (pc *prometheusChecker) checkScraperMetrics(receiver config.ComponentID, scraper config.ComponentID, scrapedMetricPoints, erroredMetricPoints int64) error{ +func (pc *prometheusChecker) checkScraperMetrics(receiver component.ID, scraper component.ID, scrapedMetricPoints, erroredMetricPoints int64) error{ scraperAttrs := attributesForScraperMetrics(receiver, scraper) return multierr.Combine( pc.checkCounter("scraper_scraped_metric_points", scrapedMetricPoints, scraperAttrs), pc.checkCounter("scraper_errored_metric_points", erroredMetricPoints, scraperAttrs)) } -func (pc *prometheusChecker) checkReceiverTraces(receiver config.ComponentID, protocol string, acceptedSpans, droppedSpans int64) error { +func (pc *prometheusChecker) checkReceiverTraces(receiver component.ID, protocol string, acceptedSpans, droppedSpans int64) error { receiverAttrs := attributesForReceiverMetrics(receiver, protocol) return multierr.Combine( pc.checkCounter("receiver_accepted_spans", acceptedSpans, receiverAttrs), @@ -152,7 +152,7 @@ func fetchPrometheusMetrics(handler http.Handler) (map[string]*io_prometheus_cli return parser.TextToMetricFamilies(rr.Body) } -func attributesForScraperMetrics(receiver config.ComponentID, scraper config.ComponentID) []attribute.KeyValue { +func attributesForScraperMetrics(receiver component.ID, scraper component.ID) []attribute.KeyValue { return []attribute.KeyValue{ attribute.String(receiverTag.Name(), receiver.String()), attribute.String(scraperTag.Name(), scraper.String()), diff --git a/obsreport/obsreporttest/otelprometheuschecker_test.go b/obsreport/obsreporttest/otelprometheuschecker_test.go index 18dccd16269..bde065ce0c0 100644 --- a/obsreport/obsreporttest/otelprometheuschecker_test.go +++ b/obsreport/obsreporttest/otelprometheuschecker_test.go @@ -47,9 +47,9 @@ func TestPromChecker(t *testing.T) { pc, err := newStubPromChecker() require.NoError(t, err) - scraper := config.NewComponentID("fakeScraper") - receiver := config.NewComponentID("fakeReceiver") - exporter := config.NewComponentID("fakeExporter") + scraper := component.NewID("fakeScraper") + receiver := component.NewID("fakeReceiver") + exporter := component.NewID("fakeExporter") transport := "fakeTransport" assert.NoError(t, From 87d68fd53e498e09b4a359b07067c6bc03fc7847 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Tue, 15 Nov 2022 15:19:38 -0500 Subject: [PATCH 07/11] address review comments --- .chloggen/obsreport-scraper.yaml | 1 - obsreport/obsreport_scraper.go | 9 +++------ obsreport/obsreporttest/obsreporttest_test.go | 3 ++- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/.chloggen/obsreport-scraper.yaml b/.chloggen/obsreport-scraper.yaml index fcfa3545eba..a8c301fac72 100644 --- a/.chloggen/obsreport-scraper.yaml +++ b/.chloggen/obsreport-scraper.yaml @@ -1,4 +1,3 @@ - # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' change_type: enhancement diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index 12513a6637c..382ff0e23dc 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -65,12 +65,8 @@ type ScraperSettings struct { } // NewScraper creates a new Scraper. -func NewScraper(cfg ScraperSettings) *Scraper { - scr, err := newScraper(cfg, featuregate.GetRegistry()) - if err != nil && cfg.ReceiverCreateSettings.Logger != nil { - cfg.ReceiverCreateSettings.Logger.Warn("Error creating an obsreport.Scraper", zap.Error(err)) - } - return scr +func NewScraper(cfg ScraperSettings) (*Scraper, error) { + return newScraper(cfg, featuregate.GetRegistry()) } // Deprecated: [v0.65.0] use NewScraper. @@ -110,6 +106,7 @@ func newScraper(cfg ScraperSettings, registry *featuregate.Registry) (*Scraper, func (s *Scraper) createOtelMetrics(cfg ScraperSettings) error { if !s.useOtelForMetrics { + s.logger.Info("not working") return nil } meter := cfg.ReceiverCreateSettings.MeterProvider.Meter(scraperScope) diff --git a/obsreport/obsreporttest/obsreporttest_test.go b/obsreport/obsreporttest/obsreporttest_test.go index 03cfa629b1e..4ba781d0e72 100644 --- a/obsreport/obsreporttest/obsreporttest_test.go +++ b/obsreport/obsreporttest/obsreporttest_test.go @@ -42,11 +42,12 @@ func TestCheckScraperMetricsViews(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - s := obsreport.NewScraper(obsreport.ScraperSettings{ + s, err := obsreport.NewScraper(obsreport.ScraperSettings{ ReceiverID: receiver, Scraper: scraper, ReceiverCreateSettings: tt.ToReceiverCreateSettings(), }) + require.NoError(t, err) ctx := s.StartMetricsOp(context.Background()) require.NotNil(t, ctx) s.EndMetricsOp(ctx, 7, nil) From f08252f986b7c3e5db6a5dc77ea99e0c7ec616ad Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Tue, 15 Nov 2022 15:23:50 -0500 Subject: [PATCH 08/11] remove unneded log --- obsreport/obsreport_scraper.go | 1 - 1 file changed, 1 deletion(-) diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index 382ff0e23dc..e66d63e0a55 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -106,7 +106,6 @@ func newScraper(cfg ScraperSettings, registry *featuregate.Registry) (*Scraper, func (s *Scraper) createOtelMetrics(cfg ScraperSettings) error { if !s.useOtelForMetrics { - s.logger.Info("not working") return nil } meter := cfg.ReceiverCreateSettings.MeterProvider.Meter(scraperScope) From 8040fb4b78ae1c5cae131b14422cc172fe86d521 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Wed, 16 Nov 2022 03:58:10 -0500 Subject: [PATCH 09/11] add newlines --- obsreport/obsreport_scraper.go | 2 +- obsreport/obsreporttest/otelprometheuschecker.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/obsreport/obsreport_scraper.go b/obsreport/obsreport_scraper.go index e66d63e0a55..5544786ade5 100644 --- a/obsreport/obsreport_scraper.go +++ b/obsreport/obsreport_scraper.go @@ -187,4 +187,4 @@ func (s *Scraper) recordMetrics(scraperCtx context.Context, numScrapedMetrics, n obsmetrics.ScraperScrapedMetricPoints.M(int64(numScrapedMetrics)), obsmetrics.ScraperErroredMetricPoints.M(int64(numErroredMetrics))) } -} \ No newline at end of file +} diff --git a/obsreport/obsreporttest/otelprometheuschecker.go b/obsreport/obsreporttest/otelprometheuschecker.go index 327b0a9b18f..332c38366b3 100644 --- a/obsreport/obsreporttest/otelprometheuschecker.go +++ b/obsreport/obsreporttest/otelprometheuschecker.go @@ -158,6 +158,7 @@ func attributesForScraperMetrics(receiver component.ID, scraper component.ID) [] attribute.String(scraperTag.Name(), scraper.String()), } } + // attributesForReceiverMetrics returns the attributes that are needed for the receiver metrics. func attributesForReceiverMetrics(receiver component.ID, transport string) []attribute.KeyValue { return []attribute.KeyValue{ From 80e28b10575ac92a7db32f0d4c609390078a32a4 Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Wed, 16 Nov 2022 22:48:21 -0500 Subject: [PATCH 10/11] run gofmt --- obsreport/obsreporttest/obsreporttest_test.go | 2 +- obsreport/obsreporttest/otelprometheuschecker.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/obsreport/obsreporttest/obsreporttest_test.go b/obsreport/obsreporttest/obsreporttest_test.go index 4ba781d0e72..75367e45f60 100644 --- a/obsreport/obsreporttest/obsreporttest_test.go +++ b/obsreport/obsreporttest/obsreporttest_test.go @@ -32,7 +32,7 @@ const ( ) var ( - scraper = component.NewID("fakeScraper") + scraper = component.NewID("fakeScraper") receiver = component.NewID("fakeReicever") exporter = component.NewID("fakeExporter") ) diff --git a/obsreport/obsreporttest/otelprometheuschecker.go b/obsreport/obsreporttest/otelprometheuschecker.go index 332c38366b3..b50225eb031 100644 --- a/obsreport/obsreporttest/otelprometheuschecker.go +++ b/obsreport/obsreporttest/otelprometheuschecker.go @@ -34,7 +34,7 @@ type prometheusChecker struct { promHandler http.Handler } -func (pc *prometheusChecker) checkScraperMetrics(receiver component.ID, scraper component.ID, scrapedMetricPoints, erroredMetricPoints int64) error{ +func (pc *prometheusChecker) checkScraperMetrics(receiver component.ID, scraper component.ID, scrapedMetricPoints, erroredMetricPoints int64) error { scraperAttrs := attributesForScraperMetrics(receiver, scraper) return multierr.Combine( pc.checkCounter("scraper_scraped_metric_points", scrapedMetricPoints, scraperAttrs), From ee515269c04fafe61e2f829a36314f55ba5eb1ad Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Thu, 17 Nov 2022 13:22:07 -0500 Subject: [PATCH 11/11] fix indenting --- obsreport/obsreport_test.go | 116 ++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 57 deletions(-) diff --git a/obsreport/obsreport_test.go b/obsreport/obsreport_test.go index c1a71524597..219e3502c5d 100644 --- a/obsreport/obsreport_test.go +++ b/obsreport/obsreport_test.go @@ -52,13 +52,13 @@ type testParams struct { err error } -func testTelemetry(t *testing.T, testFunc func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry)) { +func testTelemetry(t *testing.T, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry)) { t.Run("WithOC", func(t *testing.T) { tt, err := obsreporttest.SetupTelemetry() require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - testFunc(tt, featuregate.NewRegistry()) + testFunc(t, tt, featuregate.NewRegistry()) }) t.Run("WithOTel", func(t *testing.T) { @@ -70,12 +70,12 @@ func testTelemetry(t *testing.T, testFunc func(tt obsreporttest.TestTelemetry, r require.NoError(t, err) t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) - testFunc(tt, registry) + testFunc(t, tt, registry) }) } func TestReceiveTraceDataOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End() @@ -122,7 +122,7 @@ func TestReceiveTraceDataOp(t *testing.T) { } func TestReceiveLogsOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End() @@ -170,7 +170,7 @@ func TestReceiveLogsOp(t *testing.T) { } func TestReceiveMetricsOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End() @@ -219,64 +219,66 @@ func TestReceiveMetricsOp(t *testing.T) { } func TestScrapeMetricsDataOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { - parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) - defer parentSpan.End() + testTelemetry(t, testScrapeMetricsDataOp) +} - params := []testParams{ - {items: 23, err: partialErrFake}, - {items: 29, err: errFake}, - {items: 15, err: nil}, - } - for i := range params { - scrp, err := newScraper(ScraperSettings{ - ReceiverID: receiver, - Scraper: scraper, - ReceiverCreateSettings: tt.ToReceiverCreateSettings(), - }, registry) - require.NoError(t, err) - ctx := scrp.StartMetricsOp(parentCtx) - assert.NotNil(t, ctx) - scrp.EndMetricsOp(ctx, params[i].items, params[i].err) - } +func testScrapeMetricsDataOp(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) + defer parentSpan.End() - spans := tt.SpanRecorder.Ended() - require.Equal(t, len(params), len(spans)) + params := []testParams{ + {items: 23, err: partialErrFake}, + {items: 29, err: errFake}, + {items: 15, err: nil}, + } + for i := range params { + scrp, err := newScraper(ScraperSettings{ + ReceiverID: receiver, + Scraper: scraper, + ReceiverCreateSettings: tt.ToReceiverCreateSettings(), + }, registry) + require.NoError(t, err) + ctx := scrp.StartMetricsOp(parentCtx) + assert.NotNil(t, ctx) + scrp.EndMetricsOp(ctx, params[i].items, params[i].err) + } - var scrapedMetricPoints, erroredMetricPoints int - for i, span := range spans { - assert.Equal(t, "scraper/"+receiver.String()+"/"+scraper.String()+"/MetricsScraped", span.Name()) - switch { - case params[i].err == nil: - scrapedMetricPoints += params[i].items - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(0)}) - assert.Equal(t, codes.Unset, span.Status().Code) - case errors.Is(params[i].err, errFake): - erroredMetricPoints += params[i].items - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(0)}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - assert.Equal(t, codes.Error, span.Status().Code) - assert.Equal(t, params[i].err.Error(), span.Status().Description) + spans := tt.SpanRecorder.Ended() + require.Equal(t, len(params), len(spans)) - case errors.Is(params[i].err, partialErrFake): - scrapedMetricPoints += params[i].items - erroredMetricPoints++ - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) - require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(1)}) - assert.Equal(t, codes.Error, span.Status().Code) - assert.Equal(t, params[i].err.Error(), span.Status().Description) - default: - t.Fatalf("unexpected err param: %v", params[i].err) - } + var scrapedMetricPoints, erroredMetricPoints int + for i, span := range spans { + assert.Equal(t, "scraper/"+receiver.String()+"/"+scraper.String()+"/MetricsScraped", span.Name()) + switch { + case params[i].err == nil: + scrapedMetricPoints += params[i].items + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(0)}) + assert.Equal(t, codes.Unset, span.Status().Code) + case errors.Is(params[i].err, errFake): + erroredMetricPoints += params[i].items + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(0)}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + assert.Equal(t, codes.Error, span.Status().Code) + assert.Equal(t, params[i].err.Error(), span.Status().Description) + + case errors.Is(params[i].err, partialErrFake): + scrapedMetricPoints += params[i].items + erroredMetricPoints++ + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ScrapedMetricPointsKey, Value: attribute.Int64Value(int64(params[i].items))}) + require.Contains(t, span.Attributes(), attribute.KeyValue{Key: obsmetrics.ErroredMetricPointsKey, Value: attribute.Int64Value(1)}) + assert.Equal(t, codes.Error, span.Status().Code) + assert.Equal(t, params[i].err.Error(), span.Status().Description) + default: + t.Fatalf("unexpected err param: %v", params[i].err) } + } - require.NoError(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, int64(scrapedMetricPoints), int64(erroredMetricPoints))) - }) + require.NoError(t, obsreporttest.CheckScraperMetrics(tt, receiver, scraper, int64(scrapedMetricPoints), int64(erroredMetricPoints))) } func TestExportTraceDataOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End() @@ -325,7 +327,7 @@ func TestExportTraceDataOp(t *testing.T) { } func TestExportMetricsOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End() @@ -374,7 +376,7 @@ func TestExportMetricsOp(t *testing.T) { } func TestExportLogsOp(t *testing.T) { - testTelemetry(t, func(tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { + testTelemetry(t, func(t *testing.T, tt obsreporttest.TestTelemetry, registry *featuregate.Registry) { parentCtx, parentSpan := tt.TracerProvider.Tracer("test").Start(context.Background(), t.Name()) defer parentSpan.End()