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

Capture custom request/response properties #73

Closed
SergeyKanzhelev opened this issue Jun 3, 2019 · 8 comments
Closed

Capture custom request/response properties #73

SergeyKanzhelev opened this issue Jun 3, 2019 · 8 comments
Assignees

Comments

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Jun 3, 2019

Original: census-instrumentation/opencensus-csharp#130

The scenario:

Azure APIs return server request is in HTTP response headers and we want to put it on the span created to trace this http call.
We may argue the validity of this scenario and this might change in the future with W3C adoption, current support processes rely on this.

Also, this scenario of enhancing telemetry based on arbitrary request properties is popular among ApplicationInsights users.

While HttpClient instrumentation with DiagnosticSource allows it easily (you have the whole response object), Http listener in opencensus does not capture anything custom and does not provide extensibility points.

We have several options here

  1. Keep it as is. Ask users/library owners to implement Http listeners on their own which just enhance current span. This approach requires deep knowledge of DiagnosticSource, error-prone and extremely inefficient. It also requires special configurations done by users.
    This is a viable workaround for now.

  2. Provide hooks through DependenciesCollectorOptions like

class DependenciesCollectorOptions
{
  //...
 public  Action<HttpRequestMessage request, HttpResponseMessage response, Span span> EnhanceSpan;
}
  1. Provide a way to disable automatic instrumentation for certain calls that require special treatment and ask users/libraries to implement instrumentation themselves with OpenCensus or DiagnosticSource.
  • Provide configuration (e.g. for certain HTTP Url do not instrument). The problem of this approach is that by default 2 spans are collected and headers are injected twice. Only when a proper configuration is applied, we have something working. It's also quite inefficient and hard to configure
  • It might be possible to auto-detect the presence of another instrumentation, but it would require setting some additional tags on the Activity OR attributes on Span OR both because so far we can have any of them :( So we'll need to implement another AsyncLocal which first instrumentation will lock and the next one would not run is detects this 'lock'.

Any of above requires someone to reimplement the whole HTTP instrumentation to do something as simple as reading one custom header. And custom instrumentations would be error prone and likely would not produce high-quality data.

So I believe
(1) is workaround
(2) is a possible solution, but we might want to spend time designing better API

@SergeyKanzhelev SergeyKanzhelev added this to the Basic exporters and adapters milestone Jun 11, 2019
@SergeyKanzhelev SergeyKanzhelev modified the milestones: Basic exporters and adapters, v0.3 Oct 28, 2019
@SergeyKanzhelev SergeyKanzhelev modified the milestones: v0.3, Future Mar 4, 2020
@reyang reyang removed this from the Future milestone Jul 28, 2020
@christopher-taormina-zocdoc
Copy link
Contributor

christopher-taormina-zocdoc commented Aug 4, 2020

I've just come up against this in my usage of wanting to add many activity specific tags (e.g. head tags for http in activities, and redis key tags for redis activities). The hook solution (2) seems to best cover this use case in an easy to understand way for all instrumentation libraries, and allows for a common setup for all activities for a particular source.

Actually thinking about this a bit more, what I really wish for (and I realize this might be specific to me) are two hooks one on activity start and the other on activity end, similar to the ActivityProcessors. That way I can capture whatever extra information from the end of a call.

@cijothomas cijothomas self-assigned this Aug 19, 2020
@redondoxx
Copy link

redondoxx commented Aug 27, 2020

Hi, is option (2) feature available yet?

@cijothomas
Copy link
Member

@redondoxx The ability on enhance span using request/response is available now. Its achieved differently than this issue describes.
The request/response etc objects are stored in Activity's customproperties by the instrumentation now, so you can write a activityprocessor, to access them and enrich activity.

I'll be posting a doc with example usage today.

@redondoxx
Copy link

@cijothomas thank for your response, it will be very nice if we have some docs about this 👍 .

@cijothomas
Copy link
Member

@redondoxx Please take a look at this: #1193
I could use your feedback to this doc which is being written :)

@cijothomas cijothomas added this to the 0.5.0-beta (Beta 2) milestone Aug 28, 2020
@redondoxx
Copy link

thank i will give it a try soon :)

@cijothomas
Copy link
Member

closing as this feature is done. Docs are being working now.

@christopher-taormina-zocdoc
Copy link
Contributor

@cijothomas I think I found an issue with this, I opened a bug ticket here #1208

Yun-Ting pushed a commit to Yun-Ting/opentelemetry-dotnet that referenced this issue Oct 13, 2022
* Use complete project name for test step

* replacing project names with env var
Yun-Ting pushed a commit to Yun-Ting/opentelemetry-dotnet that referenced this issue Oct 13, 2022
* WCF instrumentation + example apps.

* Added an event source to log errors in Wcf instrumentation.

* Added content to the WCF instrumentation's README.

* Added support for the .NET Core version of WCF clients.

* Code review feedback.

* Updated code for 1.0 and changes done in main.

* Lint errors.

* Server tests and cleanup.

* Added client tests.

* Fixed some weirdness with SuppressDownstreamInstrumentation.

* Bug fixes.

* Attempting to fix lint errors.

* Code review and clean up.

* Added "wcf-" MinVerTagPrefix in csproj.

* Fun with copy-paste.

* Removed OpenTelemetry.Instrumentation.Http from wcf client examples.

* Updated link paths.

* Code review.

* Added wcf examples readme.

* Code review.

* Typo.

* Markdown lint.

* Code review.

* Code review.

* Code review.

* Code review.

* Add support for To & Via properties.

* Code review.

* Code review.

* Example readme update.

* Mirror fix from PR open-telemetry#73.

* Nits.

* Hashtable for cache.

* Code review.
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

No branches or pull requests

5 participants