[docs] Add a doc for scraping receivers #6358

dmitryax · 2022-10-19T18:23:47Z

This document adds some definitions, stability requirements and guidelines on how to update emitted metrics for scraping receivers.

codecov · 2022-10-19T18:28:54Z

Codecov Report

Base: 91.32% // Head: 91.32% // No change to project coverage 👍

Coverage data is based on head (6a67539) compared to base (b3c8744).
Patch has no changes to coverable lines.

❗ Current head 6a67539 differs from pull request most recent head ac51477. Consider uploading reports for the commit ac51477 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6358   +/-   ##
=======================================
  Coverage   91.32%   91.32%           
=======================================
  Files         241      240    -1     
  Lines       13878    13878           
=======================================
  Hits        12674    12674           
  Misses        959      959           
  Partials      245      245

Impacted Files	Coverage Δ
featuregate/gates.go	`100.00% <0.00%> (ø)`
featuregate/stage.go

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.

dmitryax · 2022-10-19T20:25:31Z

@open-telemetry/collector-contrib-approvers PTAL

codeboten

Thank you for documenting this, just a few comments/questions

docs/scraping-receivers.md

codeboten · 2022-10-19T22:31:56Z

docs/scraping-receivers.md

+
+There are two main categories of the metrics emitted by scraping receivers:
+
+- **Default metrics**: emitted by default, but can be disabled in user settings.


Any thoughts around naming these Enabled and Disabled metrics? The behaviour between these two is opposite, but the word default doesn't imply it's the opposite of optional

I'm not sure. For me, it feels like Default/Optional are more intuitive even if Default doesn't imply it's the opposite of Optional. But I'm good with Enabled/Disabled as well. The only concern is that Enabled is the same name the settings option which may be ok.

Let's keep it for now, I can rename them if there is no other feedback from @open-telemetry/collector-contrib-approvers

Another suggestion: DefaultOn / DefaultOff. It's more verbose but it makes clear that we are describing the default state only.

Enabled is problematic because then when the user disables the metric you have a "disabled Enabled metric"?

Optional is also problematic because all metrics are optional.

I agree that "default" and "optional" are a little more intuitive. I think DefaultOn and DefaultOff would be alright as well. Another idea is that we could call non-default metrics "additional" or "extra" to highlight that they aren't emitted out-of-the-box. I think that could also hint toward the purpose behind why a metric is enabled or disabled by default.

docs/scraping-receivers.md

djaglowski

Thanks for writing this up. We definitely need to pin these details down.

docs/scraping-receivers.md

evan-bradley · 2022-10-20T17:37:51Z

docs/scraping-receivers.md

+
+There are two main categories of the metrics emitted by scraping receivers:
+
+- **Default metrics**: emitted by default, but can be disabled in user settings.


I agree that "default" and "optional" are a little more intuitive. I think DefaultOn and DefaultOff would be alright as well. Another idea is that we could call non-default metrics "additional" or "extra" to highlight that they aren't emitted out-of-the-box. I think that could also hint toward the purpose behind why a metric is enabled or disabled by default.

TylerHelmuth

Thanks so much for writing this up. Tangential to this PR, we should also update https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/cmd/mdatagen/README.md to be more accurate. Maybe linking to this doc would be enough?

dmitryax · 2022-10-20T22:22:43Z

Tangential to this PR, we should also update https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/cmd/mdatagen/README.md to be more accurate. Maybe linking to this doc would be enough?

I believe https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/cmd/mdatagen/README.md should describe technical aspects of using the mdatagel tool. I'll update it once this PR is merged

dgoscn · 2022-10-24T17:46:18Z

I am facing this right this moment. If someone advances with this topic, it would be amazing

dmitryax · 2022-11-01T17:12:24Z

@djaglowski @codeboten @evan-bradley thanks for your feedback. I believe everything is addressed now. Please take another look

djaglowski

LGTM. Thanks for considering my feedback.

docs/scraping-receivers.md

andrzej-stencel

This is very educative read, thanks a lot Dmitrii.

My recent PR is a good example of applying the "Changing the emitted metrics" section. One thought related to this is that I will need to add the warning messages after this doc is accepted.

evan-bradley

Thanks for writing this up.

I left a few wordsmithing suggestions, but overall it looks good to me.

docs/scraping-receivers.md

evan-bradley · 2022-11-09T19:21:53Z

docs/scraping-receivers.md

+
+### Other changes
+
+Other breaking changes SHOULD follow similar strategies inspecting presence of `enabled` field in user settings. For 


Suggested change

Other breaking changes SHOULD follow similar strategies inspecting presence of `enabled` field in user settings. For

Other breaking changes SHOULD follow similar strategies that modify the `enabled` field in user settings. For

This correction changes the statement. We don't modify enabled field in the settings, we check if it's set only

dmitryax · 2022-11-11T22:20:45Z

@evan-bradley thanks for the review! I applied all the corrections except for the last one

dmitryax requested review from a team and bogdandrutu October 19, 2022 18:23

dmitryax force-pushed the add-doc-for-scrapers branch from 33f749d to 122bc08 Compare October 19, 2022 18:26

dmitryax added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 19, 2022

dmitryax force-pushed the add-doc-for-scrapers branch from 122bc08 to d5a64c1 Compare October 19, 2022 20:24

codeboten reviewed Oct 19, 2022

View reviewed changes

dmitryax force-pushed the add-doc-for-scrapers branch from d5a64c1 to f3bd9f0 Compare October 20, 2022 05:09

djaglowski reviewed Oct 20, 2022

View reviewed changes

evan-bradley reviewed Oct 20, 2022

View reviewed changes

TylerHelmuth reviewed Oct 20, 2022

View reviewed changes

dmitryax force-pushed the add-doc-for-scrapers branch from 9f83d43 to 251aa81 Compare October 20, 2022 22:20

dmitryax mentioned this pull request Oct 31, 2022

[Metric builder] Add external system states to be used in metric constraints open-telemetry/opentelemetry-collector-contrib#15995

Closed

dmitryax force-pushed the add-doc-for-scrapers branch from 251aa81 to e0791ab Compare November 1, 2022 17:03

djaglowski approved these changes Nov 2, 2022

View reviewed changes

dmitryax mentioned this pull request Nov 7, 2022

Project Tracking: Instrumentation Stability + Semantic Conventions Working Group open-telemetry/opentelemetry-specification#2753

Closed

andrzej-stencel reviewed Nov 8, 2022

View reviewed changes

docs/scraping-receivers.md Outdated Show resolved Hide resolved

andrzej-stencel approved these changes Nov 8, 2022

View reviewed changes

dmitryax force-pushed the add-doc-for-scrapers branch from e0791ab to 061e669 Compare November 9, 2022 17:19

evan-bradley approved these changes Nov 9, 2022

View reviewed changes

[docs] Add a doc for scraping receivers

ac51477

dmitryax force-pushed the add-doc-for-scrapers branch from 6a67539 to ac51477 Compare November 11, 2022 22:18

bogdandrutu approved these changes Nov 14, 2022

View reviewed changes

mx-psi approved these changes Nov 15, 2022

View reviewed changes

bogdandrutu merged commit dae4ebd into open-telemetry:main Nov 15, 2022

dmitryax deleted the add-doc-for-scrapers branch November 16, 2022 04:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Add a doc for scraping receivers #6358

[docs] Add a doc for scraping receivers #6358

dmitryax commented Oct 19, 2022

codecov bot commented Oct 19, 2022 •

edited

Loading

dmitryax commented Oct 19, 2022

codeboten left a comment

codeboten Oct 19, 2022

dmitryax Oct 20, 2022

djaglowski Oct 20, 2022

evan-bradley Oct 20, 2022

djaglowski left a comment

evan-bradley Oct 20, 2022

TylerHelmuth left a comment

dmitryax commented Oct 20, 2022

dgoscn commented Oct 24, 2022

dmitryax commented Nov 1, 2022

djaglowski left a comment

andrzej-stencel left a comment

evan-bradley left a comment

evan-bradley Nov 9, 2022

dmitryax Nov 11, 2022

dmitryax commented Nov 11, 2022


		There are two main categories of the metrics emitted by scraping receivers:

		- Default metrics: emitted by default, but can be disabled in user settings.


		### Other changes

		Other breaking changes SHOULD follow similar strategies inspecting presence of `enabled` field in user settings. For

[docs] Add a doc for scraping receivers #6358

[docs] Add a doc for scraping receivers #6358

Conversation

dmitryax commented Oct 19, 2022

codecov bot commented Oct 19, 2022 • edited Loading

Codecov Report

dmitryax commented Oct 19, 2022

codeboten left a comment

Choose a reason for hiding this comment

codeboten Oct 19, 2022

Choose a reason for hiding this comment

dmitryax Oct 20, 2022

Choose a reason for hiding this comment

djaglowski Oct 20, 2022

Choose a reason for hiding this comment

evan-bradley Oct 20, 2022

Choose a reason for hiding this comment

djaglowski left a comment

Choose a reason for hiding this comment

evan-bradley Oct 20, 2022

Choose a reason for hiding this comment

TylerHelmuth left a comment

Choose a reason for hiding this comment

dmitryax commented Oct 20, 2022

dgoscn commented Oct 24, 2022

dmitryax commented Nov 1, 2022

djaglowski left a comment

Choose a reason for hiding this comment

andrzej-stencel left a comment

Choose a reason for hiding this comment

evan-bradley left a comment

Choose a reason for hiding this comment

evan-bradley Nov 9, 2022

Choose a reason for hiding this comment

dmitryax Nov 11, 2022

Choose a reason for hiding this comment

dmitryax commented Nov 11, 2022

codecov bot commented Oct 19, 2022 •

edited

Loading