adaptation, stub: allow extra ttrpc client and server options.#67
Merged
fuweid merged 4 commits intocontainerd:mainfrom Feb 1, 2024
Merged
adaptation, stub: allow extra ttrpc client and server options.#67fuweid merged 4 commits intocontainerd:mainfrom
fuweid merged 4 commits intocontainerd:mainfrom
Conversation
Add WithTTRPCOptions() which can be used to pass extra client and server options to ttrpc. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add WithTTRPCOptions() which can be used to pass extra client and server options to ttrpc. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
- Coverage 64.50% 64.36% -0.14%
==========================================
Files 9 9
Lines 1834 1841 +7
==========================================
+ Hits 1183 1185 +2
- Misses 500 505 +5
Partials 151 151 ☔ View full report in Codecov by Sentry. |
Member
|
I think I like this one better? |
Member
|
+1 for this one |
Member
Author
cpuguy83
approved these changes
Feb 1, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch set adds support for passing extra ttrpc client and server options for both the runtime adaptation and the plugin stub. The primary purpose of this PR is to implement minimal plumbing which will allow the propagation of OpenTelemetry span information from runtimes to plugins, over NRI ttrpc connections,using the instrumentation interceptors implemented by https://github.com/containerd/otelttrpc.
This PR is an alternative to #68. This PR 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 [github.com/containerd/]otelttrpc).
#68 on the other hand 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. That brings in an indirect dependency on [github.com/containerd/]otelttrpc and OpenTelemetry itself.
I wanted to ask feedback about which of these alternative approaches others find more preferable, hence I filed both as draft PRs.
I think the biggest difference between these two approaches are in the blast radius of the resulting dependency changes, since #68 pulls in OpenTelemetry as an indirect dependency via otelttrpc.