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 support for propagating OpenTelemetry trace span information over NRI ttrpc connection. #68

Closed
wants to merge 3 commits into from

Conversation

klihub
Copy link
Member

@klihub klihub commented Jan 30, 2024

This patch set implements extra runtime/adaptation and plugin/stub NRI options which enable the propagation of OpenTelemetry trace span information over ttrpc connections.

This PR is an alternative to #67. This PR introduces two boolean options which tell NRI whether it internally should set up the necessary ttrpc options to enable otel trace span propagation over ttrpc. This brings in an indirect dependency on [github.com/containerd/]otelttrpc and OpenTelemetry itself.

#67 on the other hand simply opens up the ttrpc client and server used by NRI for extra ttrpc options. This allows one to pass in the necessary options to enable otel trace span propagation (essentially using the same otelttrpc instrumenting ttrpc interceptors as used here), both in NRI enabled runtimes and NRI plugins.

I wanted to ask feedback about which of these alternative approaches others find more preferable, hence I filed both as draft PRs.

Add a WithOpenTelemetry() option which turns on OpenTelemetry
tracing instrumentation for called and served ttrpc requests
in plugins/the stub.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add a WithOpenTelemetry() option which turns on OpenTelemetry
tracing instrumentation for called and served ttrpc requests
in the runtime/adaptation.

Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
@codecov-commenter
Copy link

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (6f5a4d2) 64.50% compared to head (858e504) 64.11%.

❗ Current head 858e504 differs from pull request most recent head 3090355. Consider uploading reports for the commit 3090355 to get more accurate results

Files Patch % Lines
pkg/adaptation/plugin.go 45.45% 10 Missing and 2 partials ⚠️
pkg/adaptation/adaptation.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
- Coverage   64.50%   64.11%   -0.40%     
==========================================
  Files           9        9              
  Lines        1834     1853      +19     
==========================================
+ Hits         1183     1188       +5     
- Misses        500      512      +12     
- Partials      151      153       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@klihub
Copy link
Member Author

klihub commented Jan 31, 2024

Closing in favor of #67.

@klihub klihub closed this Jan 31, 2024
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.

2 participants