Http Instrumentation: Fix gRPC causing issues retrieving request & response objects #2698
Conversation
…ation being sent different payloads.
Codecov Report
@@ Coverage Diff @@
## main #2698 +/- ##
==========================================
+ Coverage 83.80% 83.81% +0.01%
==========================================
Files 250 250
Lines 8828 8830 +2
==========================================
+ Hits 7398 7401 +3
+ Misses 1430 1429 -1
|
| Assert.True(fetch.TryFetch(new PayloadTypeB(), out propertyValue)); | ||
| Assert.Equal("B", propertyValue); |
There was a problem hiding this comment.
Interesting, so if most of the fetches are against PayloadTypeB but the first invocation for a process is against PayloadTypeA there'll always be a fetch first assuming A and then another for B. Probably not a huge deal overhead-wise, but just explicitly calling it out.
There was a problem hiding this comment.
Ya if you look at the issue it looks like some kind of Azure Functions thing where it always does a gRPC call first. So for native HttpClient invocations in that environment, it is going to pay the penalty of a failed is check and then an extra virtual dispatch on each call. Very minor hit, but a hit nonetheless. Not much we can do about it. If we instead did a dictionary lookup, we would have to pay for a hash + lookup for all invocations even in cases where only one payload is ever used. So this seemed like the lesser evil. Kind of lame this gRPC path uses the same diagnostic source but with different payload types.
| @@ -2,6 +2,10 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| * Fixed an issue with `Filter` and `Enrich` callbacks not firing under certain | |||
| conditions when gRPC is used | |||
| ([#2698](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2698)) | |||
There was a problem hiding this comment.
I think you meant to link the PR not the issue
(#2698)
Fixes #2652
Changes
PropertyFetcheris given a different payload type than what was used during construction, it will now build an inner instance and defer to that. We could have also looked up the correctPropertyFetcherfrom some kind of dictionary (by payload type), but I went with this design to keep the common path (where only a single payload type is used) fast/lock-free.TODOs
CHANGELOG.mdupdated for non-trivial changes