Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrument obsreport.Processor #6607

Conversation

moh-osman3
Copy link
Contributor

Description:

This PR instruments obsreport.Processor with otel go. This is a followup PR that is based on similar changes made for obsreport.Receiver: #6222

Link to tracking Issue: Part of #816

Testing:
Ran contrib collector with memory_limiter processor and prometheusreceiver + sent traces from demo client and confirmed that new metrics show up. Tested both with feature gate disabled and enabled.

From output of $ curl localhost:8888/metrics

# HELP otelcol_processor_accepted_metric_points Number of metric points successfully pushed into the next component in the pipeline.
# TYPE otelcol_processor_accepted_metric_points counter
otelcol_processor_accepted_metric_points{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 298
# HELP otelcol_processor_accepted_spans Number of spans successfully pushed into the next component in the pipeline.
# TYPE otelcol_processor_accepted_spans counter
otelcol_processor_accepted_spans{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 84
# HELP otelcol_processor_dropped_metric_points Number of metric points that were dropped.
# TYPE otelcol_processor_dropped_metric_points counter
otelcol_processor_dropped_metric_points{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 0
# HELP otelcol_processor_dropped_spans Number of spans that were dropped.
# TYPE otelcol_processor_dropped_spans counter
otelcol_processor_dropped_spans{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 0
# HELP otelcol_processor_refused_metric_points Number of metric points that were rejected by the next component in the pipeline.
# TYPE otelcol_processor_refused_metric_points counter
otelcol_processor_refused_metric_points{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 0
# HELP otelcol_processor_refused_spans Number of spans that were rejected by the next component in the pipeline.
# TYPE otelcol_processor_refused_spans counter
otelcol_processor_refused_spans{processor="memory_limiter",service_instance_id="d957e470-f4a9-4448-b0b9-e2a2e98a2b53",service_name="otelcontribcol",service_version="v0.63.0-48-ge9099f1c57"} 0

@moh-osman3 moh-osman3 requested a review from a team as a code owner November 22, 2022 19:06
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Base: 91.07% // Head: 91.21% // Increases project coverage by +0.14% 🎉

Coverage data is based on head (5d7363a) compared to base (d9b85d3).
Patch coverage: 97.64% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6607      +/-   ##
==========================================
+ Coverage   91.07%   91.21%   +0.14%     
==========================================
  Files         242      244       +2     
  Lines       14054    14165     +111     
==========================================
+ Hits        12800    12921     +121     
+ Misses       1004      995       -9     
+ Partials      250      249       -1     
Impacted Files Coverage Δ
component/config.go 87.50% <95.12%> (+87.50%) ⬆️
obsreport/obsreport_processor.go 95.00% <97.90%> (+0.30%) ⬆️
obsreport/obsreporttest/obsreporttest.go 88.46% <100.00%> (+4.84%) ⬆️
obsreport/obsreporttest/otelprometheuschecker.go 95.12% <100.00%> (+1.06%) ⬆️
service/collector.go 76.31% <0.00%> (ø)
config/connector.go 33.33% <0.00%> (ø)
connector/connector.go 66.66% <0.00%> (ø)
component/component.go 63.82% <0.00%> (+1.60%) ⬆️
component/exporter.go 100.00% <0.00%> (+3.92%) ⬆️
component/receiver.go 100.00% <0.00%> (+3.92%) ⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

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

# (Optional) One or more lines of additional information to render under the primary note.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this if it's unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! Removed.

return newProcessor(cfg, featuregate.GetRegistry())
}

// Deprecated: [v0.65.0] use NewProcessor.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR will probably land after we release 0.66.0 (happening now), so, this needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out! Removed this deprecated function now.

@jpkrohling
Copy link
Member

jpkrohling commented Nov 23, 2022

@paivagustavo, can you review this one as the author of the PR that this one is using as reference?

Copy link
Member

@paivagustavo paivagustavo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @moh-osman3, I've added one small note regarding consistency with other otel instrumentations.

Sorry for taking too long, I was away in vacation :)

obsreport/obsreport_processor.go Outdated Show resolved Hide resolved
@bogdandrutu bogdandrutu merged commit 4ff1ff3 into open-telemetry:main Nov 28, 2022
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Nov 28, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Dec 1, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Dec 5, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Dec 5, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Dec 6, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu pushed a commit to bogdandrutu/opentelemetry-collector that referenced this pull request Dec 6, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
bogdandrutu added a commit that referenced this pull request Dec 6, 2022
…and] to collector package (#6608)

* Instrument obsreport.Processor (#6607)

* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics

* Update otelcol/moved.go

Co-authored-by: Alex Boten <alex@boten.ca>

* Update service/collector.go

Co-authored-by: Alex Boten <alex@boten.ca>

Co-authored-by: Moh Osman <59479562+moh-osman3@users.noreply.github.com>
Co-authored-by: Alex Boten <alex@boten.ca>
jaronoff97 added a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
…and] to collector package (open-telemetry#6608)

* Instrument obsreport.Processor (open-telemetry#6607)

* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics

* Update otelcol/moved.go

Co-authored-by: Alex Boten <alex@boten.ca>

* Update service/collector.go

Co-authored-by: Alex Boten <alex@boten.ca>

Co-authored-by: Moh Osman <59479562+moh-osman3@users.noreply.github.com>
Co-authored-by: Alex Boten <alex@boten.ca>
jaronoff97 added a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics
jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022
…and] to collector package (open-telemetry#6608)

* Instrument obsreport.Processor (open-telemetry#6607)

* Instrument obsreport.Processor

* add processor promchecker tests

* add chloggen

* run make genpdata and fix linter errors

* address review feedback and retest

* pass cfg into createOtelMetrics

* Update otelcol/moved.go

Co-authored-by: Alex Boten <alex@boten.ca>

* Update service/collector.go

Co-authored-by: Alex Boten <alex@boten.ca>

Co-authored-by: Moh Osman <59479562+moh-osman3@users.noreply.github.com>
Co-authored-by: Alex Boten <alex@boten.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants