Design document for otelhttpconv package #6859

dmathieu · 2025-02-28T14:42:53Z

This provides a design document for a new instrumentation, otelhttpconv that would serve as a meta instrumentation for all HTTP packages to more easily share their behavior.

codecov · 2025-02-28T14:53:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.6%. Comparing base (dbaba60) to head (cfd8b48).

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #6859   +/-   ##
=====================================
  Coverage   75.6%   75.6%           
=====================================
  Files        207     207           
  Lines      19354   19354           
=====================================
+ Hits       14643   14646    +3     
+ Misses      4275    4273    -2     
+ Partials     436     435    -1

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dmathieu · 2025-03-04T13:16:45Z

cc @akats7 @scorpionknifes as code owners of HTTP implementations

instrumentation/net/http/otelhttpconv/DESIGN.md

Co-authored-by: Robert Pająk <[email protected]>

XSAM

Could you add an example for a certain existing HTTP instrumentation to describe how otelhttpconv is actually being used in HTTP instrumentation? This could also give other authors more clues about how to leverage otelhttpconv to reduce code repetition.

dmathieu · 2025-03-11T09:32:58Z

I've added a sample implementation of an http.Handler to show what it would look like for implementations.
I'm happy to provide a full implementation of otelhttpconv as well, to make this code work. But I'd rather do that when we're nearing consensus to avoid having to change things too many times.

instrumentation/net/http/otelhttpconv/DESIGN.md

Co-authored-by: Sam Xie <[email protected]>

instrumentation/net/http/otelhttpconv/DESIGN.md

pellared · 2025-03-17T15:42:10Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+
+	// NewResponseWrapper wraps additional data into the http.ResponseWriter,
+	// such as duration, status code and quantity of bytes read.
+	rww := otelhttpconv.NewResponseWrapper(w)


Isn't this coupled to otelhttpconv.NewHTTPConv?
Are we sure that it would work with other custom semantic conventions?

I think this goes with your other comment. If we make the body and response wrapper structs and internalize their logic then, this will indeed change.

So, even with structs for the wrappers, I think this design makes sense.
Building a response wrapper from an HTTPConv would be strange, as then each implementations would need to know how to build a wrapper as well.

as then each implementations would need to know how to build a wrapper as well.

But wouldn't the wrapper functionalities by detriment by what httpconv.Record is supposed to do? I feel like those structures are coupled to each other. E.g. I guess that for some httpconv the reponse needs BytesWritten but for other it is unnecessary.

instrumentation/net/http/otelhttpconv/DESIGN.md

pellared · 2025-03-17T16:35:08Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+
+// Client provides an interface for HTTP client instrumentations to set the
+// proper semantic convention attributes and metrics into their data.
+type Client interface {


I think it would be good to have some "Rationale" or/and "Rejected Proposals" explaining the pros and cons behind the decision making.

For instance, why cannot we simply have a function a struct more or less like

func RecordClient(r *http.Request, params RecordClientParameters) type RecordClientParameters struct { Tracer trace.Tracer Meter metric.Meter Error error Response *http.Response Duration time.Time }

I've added an Alternatives section.

pellared

It would be nice to have a working prototype as a separate draft PR.

dmathieu · 2025-03-18T11:01:30Z

It would be nice to have a working prototype as a separate draft PR.

I intend to do that. But I'd rather have the spec stabilize first, to avoid rewriting it several times.

pellared · 2025-03-18T11:39:26Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+These interfaces are kept relatively simple on purpose, so they can be used as
+foundations if folks wish to extend them with their own actual implementations.


What would be a problem to use a struct? They can still be used as foundations. The users can always create their own interfaces if they need to support different implementation.

How would you do that?
If we provide the following API:

otelhttp.NewTransport(http.DefaultTransport, otelhttp.WithHTTPConv(myCustomImplementation{}))

And otelhttp.WithHTTPConv takes a struct, you can't extend it. It has to be our implementation.

otelhttp would define the interface as this is the package that accepts input and not otelhttpconv which provides an implementation.

otelhttp should not expose otelhttpconv types

Let's discuss this during today's SIG meeting. We have different views here.

While otelhttp would definitely use this package, the idea here is that other packages which currently use the generated semconv package would too.
Then, they need the types exposed.

Let's discuss this during today's SIG meeting. We have different views here.

👍

the idea here is that other packages which currently use the generated semconv package would too.

Sure but these can be struct and not interface.

pellared · 2025-03-18T11:43:12Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+Once stable, we will not be able to change those interfaces anymore (adding,
+renaming or deleting methods), as that would break backwards compatibility with
+implementations.


Then what is our plan if we would like to add a new functionality?

Same as for the interfaces in the SDK. We either introduce a new interface, or we release a v2.

With a struct we could simply add new fields/method.

pellared · 2025-03-18T11:43:45Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+
+### Implementations
+
+We will provide one official implementation of the described interfaces.


What is the point of exposing an interface if we want to provide only one implementation?

So that folks can provide their own implementation.
Also, I do mention that we may provide additional implementations later on, for unstable conventions for example.

The places where we want to allow passing custom implementations should define the interface (e.g. otelhttp). otelhttpconv does not accept any custom implementation.

Reference: https://go.dev/wiki/CodeReviewComments#interfaces

To be honest, I am not even sure if we need to support custom semantic conventions (we never did that).

pellared · 2025-03-18T13:23:33Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+		tracer:      otel.Tracer("http"),
+		propagators: otel.GetTextMapPropagator(),
+		meter:       otel.Meter("http"),
+		httpconv:    otelhttpconv.NewHTTPConv(otel.Tracer("httpconv"), otel.Meter("httpconv")),


NewHTTPConv is not defined in the design.

Sorry, that seemed clear to me. It creates an instance of our official implementation of the interface.
Since the implementation itself isn't in the design, I didn't add this method.

pellared · 2025-03-18T13:39:40Z

instrumentation/net/http/otelhttpconv/DESIGN.md

+### Compatibility
+
+These interfaces are kept relatively simple on purpose, so they can be used as
+foundations if folks wish to extend them with their own actual implementations.


Do we really want to support custom semconv implementations?
I think it is an overkill (and footgun). Shouldn't we just focus on OTel SemConv?
People not happy with OTel SemConv can create their own instrumentation libraries.
I propose that otelhttpconv has "concrete" implementations (functions, structs).

If we want to ever provide customization of semconv, I would rather add it on the instrumentation library layer as this is the API that the user is interacting with. otelhttpconv should be used "internally" by instrumentation libraries.

I think being able to support custom semantic conventions is just a side effect of the modularity. The real interesting thing here would be to be able to support unstable semantic conventions, with no breaking changes on the code that supports stable ones.

I find it hard to evaluate without code and examples.
Unstable semantic conventions could be also controlled by env vars.

first draft for otelhttpconv design document

800942f

dmathieu added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Feb 28, 2025

fix typo

e7e8c88

dmathieu added 4 commits March 4, 2025 12:12

add server design

87a12f6

fix indentation

cdd2a52

some copy editing

4895dc2

Merge branch 'main' into otelhttpconv-design

65ba119

dmathieu marked this pull request as ready for review March 4, 2025 13:15

dmathieu requested a review from a team as a code owner March 4, 2025 13:15

dmathieu mentioned this pull request Mar 5, 2025

Move otelhttp/internal/semconvutil into otelhttp #4580

Open

pellared reviewed Mar 5, 2025

View reviewed changes

instrumentation/net/http/otelhttpconv/DESIGN.md Outdated Show resolved Hide resolved

instrumentation/net/http/otelhttpconv/DESIGN.md Outdated Show resolved Hide resolved

dmathieu and others added 2 commits March 5, 2025 13:51

Update instrumentation/net/http/otelhttpconv/DESIGN.md

fbc47f1

Co-authored-by: Robert Pająk <[email protected]>

specify the package will be used by other packages

63f9061

XSAM reviewed Mar 10, 2025

View reviewed changes

dmathieu added 2 commits March 11, 2025 09:52

Merge branch 'main' into otelhttpconv-design

96054d1

add a sample use in an http.Handler

4084bc3

boekkooi-impossiblecloud reviewed Mar 11, 2025

View reviewed changes

dmathieu added 5 commits March 11, 2025 15:07

reference responsewriter directly, not through pointer

5efaca9

add BytesWritten and Error

2e58049

split ResponseWrapper and BodyWrapper into two interfaces

f51a9fa

fix spelling

d96c7ec

Merge branch 'main' into otelhttpconv-design

77d428d

XSAM approved these changes Mar 14, 2025

View reviewed changes

instrumentation/net/http/otelhttpconv/DESIGN.md Show resolved Hide resolved

Update instrumentation/net/http/otelhttpconv/DESIGN.md

1468bfd

Co-authored-by: Sam Xie <[email protected]>

pellared reviewed Mar 17, 2025

View reviewed changes

instrumentation/net/http/otelhttpconv/DESIGN.md Outdated Show resolved Hide resolved

pellared reviewed Mar 17, 2025

View reviewed changes

instrumentation/net/http/otelhttpconv/DESIGN.md Outdated Show resolved Hide resolved

pellared reviewed Mar 17, 2025

View reviewed changes

dmathieu added 4 commits March 18, 2025 11:39

Merge branch 'main' into otelhttpconv-design

9231a54

internalize the request and response wrappers

fc9dd8c

add compatibility section

c7c2fe6

merge RecordMetrics and RecordSpan

2788ed3

add alternatives section

cfd8b48

pellared reviewed Mar 18, 2025

View reviewed changes

dmathieu mentioned this pull request Mar 24, 2025

Gin instrumentation: support reading Client IP from custom headers, and make sure proxy is trusted #6095

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design document for otelhttpconv package #6859

Design document for otelhttpconv package #6859

dmathieu commented Feb 28, 2025 •

edited

Loading

codecov bot commented Feb 28, 2025 •

edited

Loading

dmathieu commented Mar 4, 2025

XSAM left a comment

dmathieu commented Mar 11, 2025

pellared Mar 17, 2025

dmathieu Mar 17, 2025

dmathieu Mar 18, 2025

pellared Mar 18, 2025

pellared Mar 17, 2025 •

edited

Loading

dmathieu Mar 18, 2025

pellared left a comment

dmathieu commented Mar 18, 2025

pellared Mar 18, 2025

dmathieu Mar 18, 2025

pellared Mar 18, 2025

dmathieu Mar 20, 2025

pellared Mar 20, 2025 •

edited

Loading

pellared Mar 18, 2025

dmathieu Mar 18, 2025

pellared Mar 18, 2025

pellared Mar 18, 2025

dmathieu Mar 18, 2025

pellared Mar 18, 2025 •

edited

Loading

pellared Mar 18, 2025

dmathieu Mar 18, 2025

pellared Mar 18, 2025 •

edited

Loading

dmathieu Mar 18, 2025

pellared Mar 20, 2025 •

edited

Loading

		These interfaces are kept relatively simple on purpose, so they can be used as
		foundations if folks wish to extend them with their own actual implementations.


		### Implementations

		We will provide one official implementation of the described interfaces.

Design document for otelhttpconv package #6859

Are you sure you want to change the base?

Design document for otelhttpconv package #6859

Conversation

dmathieu commented Feb 28, 2025 • edited Loading

codecov bot commented Feb 28, 2025 • edited Loading

Codecov Report

dmathieu commented Mar 4, 2025

XSAM left a comment

Choose a reason for hiding this comment

dmathieu commented Mar 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared left a comment

Choose a reason for hiding this comment

dmathieu commented Mar 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pellared Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

dmathieu commented Feb 28, 2025 •

edited

Loading

codecov bot commented Feb 28, 2025 •

edited

Loading

pellared Mar 17, 2025 •

edited

Loading

pellared Mar 20, 2025 •

edited

Loading

pellared Mar 18, 2025 •

edited

Loading

pellared Mar 18, 2025 •

edited

Loading

pellared Mar 20, 2025 •

edited

Loading