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

Add dynamic metric configuration support #751

Merged
merged 8 commits into from
Aug 13, 2020

Conversation

wtong98
Copy link

@wtong98 wtong98 commented Jul 29, 2020

NOTE: this is an experimental feature, not blocking for GA

Follows from this otep

Goal

We introduce a system for dynamically configuring metric collection schedules. The system includes three major changes:

  • collection periods may be changed during runtime via a configuration service
  • collection periods are keyed per-metric, rather than collecting all metrics
  • a configuration service may read config data from a local file or a remote (potentially third-party) backend

An example implementation is almost completed and will be sent to the collector-contrib and go-contrib repos. In the meantime, we'd love to hear your thoughts on this specification and the related protocol!

About

We're an intern group at Google. Members include:

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 1, 2020

CLA Check
The committers are authorized under a signed CLA.

Copy link
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

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

This is great. /I definitely support this and the notion of dynamic configuration for metric instruments and for metric and tracing SDKs in general. My only slight concern is that implementing dynamic or multiple Metric collection periods isn't the easiest place to start, but it's definitely good functionality to have. 😀

@carlosalberto carlosalberto added area:sdk Related to the SDK release:after-ga Not required before GA release, and not going to work on before GA spec:metrics Related to the specification/metrics directory labels Aug 4, 2020
@jkwatson
Copy link
Contributor

jkwatson commented Aug 4, 2020

I think that per-metric push-intervals should be de-scoped for GA. That's a significant ask to be built in every language, and I would recommend removing it for now.

@wtong98
Copy link
Author

wtong98 commented Aug 4, 2020

@jkwatson Definitely, the per-metric collection periods are a sweeping change, and will likely require substantial work (although for the Go SDK, we came up with a relatively contained way to introduce the change -- will make a PR on go-contrib soon). Overall, I'd be fine if the spec remained experimental and no hard changes included at all until after GA. The plan on our end has been to introduce prototype implementations to the go-contrib and collector-contrib repos, and if feedback is positive and it seems like a good idea, to migrate changes into the main project (probably after GA). Does that sound like a good way forward?

Copy link
Member

@huyan0 huyan0 left a comment

Choose a reason for hiding this comment

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

I think its a really interesting idea. Thanks.

@jmacd
Copy link
Contributor

jmacd commented Aug 6, 2020

I believe we decided to call this an experiment so that it will not be a requirement for SDKs to implement. Given that, I don't feel the objections to the specific kind of configuration being delivered are very important.

@SergeyKanzhelev SergeyKanzhelev self-assigned this Aug 10, 2020
@bogdandrutu bogdandrutu self-assigned this Aug 12, 2020
@bogdandrutu bogdandrutu requested review from a team and carlosalberto August 12, 2020 20:41
@bogdandrutu bogdandrutu reopened this Aug 13, 2020
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
* Translate gdoc to markdown

* Add links and missing image

* resize image

* Upsize image

* Rename experimental/metric-config-service.md to experimental/metrics/config-service.md

* Update config-service.md

Co-authored-by: Ming Chen <[email protected]>
Co-authored-by: Bogdan Drutu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK release:after-ga Not required before GA release, and not going to work on before GA spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants