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

Stable release of HTTP Instrumentations #4376

Closed
12 of 13 tasks
cijothomas opened this issue Apr 7, 2023 · 2 comments
Closed
12 of 13 tasks

Stable release of HTTP Instrumentations #4376

cijothomas opened this issue Apr 7, 2023 · 2 comments

Comments

@cijothomas
Copy link
Member

cijothomas commented Apr 7, 2023

{Draft, still editing, adding links to other issues}

Tracking issue for covering the stable release of HTTP Instrumentations for Server and Client (i.e. Asp.Net Core, HttpClient). Most issues
already exist, so will be linking to them. This is just to track the overall status.

  • Pre-req - OTel Semantic Conventions Stable.
  • Required/Recommended/Optional attributes - How would users enable/disable. See Update instrumentation libraries with latest semantic convention spec #3373 (comment) Update: Default set of attributes for HttpClient instrumentation (trace and metric) #4993 and Default set of attributes for ASP.NET Core instrumentation (trace and metric) #4992 includes default set of attributes instrumentation will add. Ability to configure required/recommended attributes will not be provided in initial stable release.
  • Tracing and Metrics in one package - if Metrics convention is not stable, but Tracing - how to handle this in single package.
    Edit on 5/17/2023 - Metrics are also expected to be stable along with Tracing. Update 10/19 Tracing and Metrics will be released in a single package.
  • Some instrumentation enables HTTP and RPC. Keep same or separate? (semantic conventions are likely moving in different pace): update 10/19 Grpc server instrumentation will be supported under experimental feature flag for stable release of instrumentation library.
  • Lifecycle management of the instrumentation libraries when there are multiple instances of SDK (especially for Metrics instrumentation)
  • Public API review. (Enrich* had breaking change in the past, might need similar for Filter?) Update : We will not do breaking changes for filter API.
  • Finalize target frameworks. (Testing netstandard2.0 code path etc.)
  • Triage and fix all blocking bugs
  • Performance - benchmarks vs RPS drop using bombardier etc., Stress Tests to ensure no leaks, etc.
  • Some instrumentations have an undesired dependency on SDK, not API. Fixing this might require additional work in API/SDK. Update 11/29: SDk dependency has been removed from both the instrumentations.
  • Propagators from OTel vs .NET itself. How to ensure smooth/least disruptive migration to the one from DS itself. Update 10/23 : This will be addressed in .NET9.0 when framework supports native trace instrumentation.
  • Document what does start and stop of Activity mean. (and there define what does the duration measures) Document the exact definition of durations #3277
  • Rename the options classes to include signal (Trace/Metric) in their name.
@vishweshbankwar
Copy link
Member

Related #4484

@vishweshbankwar
Copy link
Member

vishweshbankwar commented Oct 19, 2023

Adding few more things to do/consider:

TODOs

ASP.NET Core

  1. Update span name as per spec: (This includes http.route tag value)
    https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=Instrumentation%20MUST%20NOT%20default%20to%20using%20URI%20path%20as%20span%20name%2C%20but%20MAY%20provide%20hooks%20to%20allow%20custom%20logic%20to%20override%20the%20default%20span%20name
    https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=HTTP%20server%20span%20names%20SHOULD,names%20SHOULD%20be%20%7Bmethod%7D

  2. Add error.type attribute: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=%5B1%5D%3A%20If,specific%20error%20identifier

HttpClient:

  1. Update span name as per spec:
    https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=HTTP%20client%20spans,MUST%20be%20HTTP
    https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=Instrumentation%20MUST%20NOT%20default%20to%20using%20URI%20path%20as%20span%20name%2C%20but%20MAY%20provide%20hooks%20to%20allow%20custom%20logic%20to%20override%20the%20default%20span%20name

  2. Add error.type attribute: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=%5B1%5D%3A%20If,specific%20error%20identifier

Things to clarify if needed before GA:

  1. Validate if following attributes are needed:
    http.request.method_original --> https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=Required-,http.request.method_original,Conditionally%20Required%3A%20%5B4%5D,-http.response.body
    http.request.body.size
    http.response.body.size
  2. Password sanitizing: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=%5B5%5D%3A%20For,for%20sanitizing%20purposes update 10/24 This is not required for stable release
  3. Do we need to address http.resend_count: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#http-request-retries-and-redirects:~:text=Because%20of%20the%20potential%20for%20confusion%20around%20this%2C%20HTTP%20client%20library%20instrumentations%20SHOULD%20document%20their%20behavior%20around%20ending%20HTTP%20client%20spans.

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

2 participants