[Instrumentation.MassTransit] Masstransit enricher #696
Conversation
nicolasderycke
changed the title
|
@cijothomas : I created a new pull request with the latest version in it. |
utpilla
added
the
comp:instrumentation.masstransit
|
Adding @alexvaluyskiy, who is the component owner to review the PR. |
| .AddMassTransitInstrumentation(options => | ||
| { | ||
| // Enable enriching an activity after it is created. | ||
| options.Enrich = (activity, eventName, rawObject) => |
There was a problem hiding this comment.
An example that actually makes use of the rawObject passed would be more helpful.
src/OpenTelemetry.Instrumentation.MassTransit/Implementation/MassTransitDiagnosticListener.cs
Outdated
Show resolved
Hide resolved
|
|
||
| try | ||
| { | ||
| this.options.Enrich?.Invoke(activity, "OnStartActivity", payload); |
There was a problem hiding this comment.
we are moving to a new approach wherever feasible: open-telemetry/opentelemetry-dotnet#3792
Please check this, and see if this can be done here too.
There was a problem hiding this comment.
@nicolasderycke, I see some new commits after Cijo comments, but I think that you have missed part of this.
In Http instrumentation there where one Enrichment method which returns in callback string with the event name (OnStartActivity, OnStopActivity, OnException).
After changes we have 3 separate methods
options.EnrichWithHttpRequestMessage
options.EnrichWithHttpResponseMessage
options.EnrichWithException
Any chance to introduce something like that here?
There was a problem hiding this comment.
@nicolasderycke Are you working on these suggestions?
There was a problem hiding this comment.
I will review these comments this week
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #696 +/- ##
==========================================
+ Coverage 77.87% 77.98% +0.11%
==========================================
Files 176 176
Lines 5301 5305 +4
==========================================
+ Hits 4128 4137 +9
+ Misses 1173 1168 -5
|
|
@nicolasderycke, we have a request from MassTransit maintaners to deprecate package as it is not needed any longer if you are using MassTransit 8.0.0+. We would like to do it shortly. We could make one more release for older versions with these changes if needed. What is your opinion about it? PR to deprecate package #788. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Added enricher for Masstransit activity