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

stats/opentelemetry: Add CSM Observability API #7277

Merged
merged 5 commits into from
May 29, 2024
Merged

Conversation

zasweq
Copy link
Contributor

@zasweq zasweq commented May 29, 2024

RELEASE NOTES: N/A

@zasweq zasweq added the Type: Feature New features or improvements in behavior label May 29, 2024
@zasweq zasweq added this to the 1.65 Release milestone May 29, 2024
@zasweq zasweq requested a review from dfawley May 29, 2024 20:55
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Looks pretty good - mostly just nits.

stats/opentelemetry/csm/observability.go Outdated Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Outdated Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Outdated Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Outdated Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Outdated Show resolved Hide resolved
stats/opentelemetry/csm/observability.go Show resolved Hide resolved
otelinternal "google.golang.org/grpc/stats/opentelemetry/internal"
)

// EnableObservability sets up CSM EnableObservability for the binary globally.
Copy link
Member

Choose a reason for hiding this comment

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

CSM EnableObservability? What's that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol it switched this docstring.


// EnableObservability sets up CSM EnableObservability for the binary globally.
//
// The CSM Plugin Option is instantiated with local labels and metadata exchange
Copy link
Member

Choose a reason for hiding this comment

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

The CSM Stats Plugin? Metrics exporter? ??? Let's not put "plugin option" in our docstrings, though, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose "Stats Plugin".

@dfawley dfawley assigned zasweq and unassigned dfawley May 29, 2024
@zasweq zasweq merged commit fe82db4 into grpc:master May 29, 2024
11 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants