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 protocol to Metric Configuration Service #183

Merged
merged 4 commits into from
Aug 13, 2020

Conversation

wtong98
Copy link
Contributor

@wtong98 wtong98 commented Jul 29, 2020

Changes

Goal

We introduce a new protocol for dynamically configuring metric collection schedules. It follows from this OTEP.

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 protocol and the related specification!

About

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

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Please split the PR:

  1. Delete all generated files
  2. Rest of changes for the experimental feature

@bogdandrutu bogdandrutu dismissed their stale review July 29, 2020 18:35

I pressed the magic button to update the branch and things are good now

@wtong98
Copy link
Contributor Author

wtong98 commented Jul 29, 2020

@bogdandrutu Great thanks! The local setup was a little weird, since we based our changes off the 0.4.0 release to avoid dependency problems in our prototypes. Sorry I missed your earlier message! I went ahead and straightened out the history for this PR and force-pushed the changes.

Copy link

@anuraaga anuraaga 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!

Comment on lines 26 to 27
// MetricConfig is a service that enables updating metric schedules, trace
// parameters, and other configurations on the SDK without having to restart the

Choose a reason for hiding this comment

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

Suggested change
// MetricConfig is a service that enables updating metric schedules, trace
// parameters, and other configurations on the SDK without having to restart the
// MetricConfig is a service that enables updating metric schedules and other configurations on the SDK without having to restart the

Looks like the scope was narrowed to just metrics which makes sense


syntax = "proto3";

package opentelemetry.proto.experimental.metricconfigservice;
Copy link
Member

Choose a reason for hiding this comment

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

add this in:
"experimental/metrics/configservice"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@SergeyKanzhelev
Copy link
Member

Thank you for initiating the experimental folder!

@bogdandrutu bogdandrutu merged commit e53be03 into open-telemetry:master Aug 13, 2020
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.

6 participants