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

Metrics shipped with http client cannot be augmented with custom attributes #5084

Closed
zailic opened this issue Feb 13, 2024 · 1 comment
Closed
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp

Comments

@zailic
Copy link
Contributor

zailic commented Feb 13, 2024

Description

At the moment it seems there's no way to add custom attributes to the provided metrics within instrumented http client. In the current implementation the metric attributes are added through a Labeler component which is designed to live within context objects tree.
However, looking at the current implementation I'm spotting two issues:

if t.clientTrace != nil {
ctx = httptrace.WithClientTrace(ctx, t.clientTrace(ctx))
}
labeler := &Labeler{}
ctx = injectLabeler(ctx, labeler)
r = r.Clone(ctx) // According to RoundTripper spec, we shouldn't modify the origin request.
// use a body wrapper to determine the request size

  1. Line 151 - a new Labeler is created instead of getting from context object (in order not to lose the potential attributes previously created and carried through the context)
  2. Line 152 - the injectLabeler method cannot be used outside the otelhttp module so actually this makes impossbile the adding of the Labeler component in the high levels of context object.

Environment

  • otelhttp version: latest

Expected behavior

This seems like an easy fix:

  • obtain the Labeler via LabelerFromContext method:
    // LabelerFromContext retrieves a Labeler instance from the provided context if
    // one is available. If no Labeler was found in the provided context a new, empty
    // Labeler is returned and the second return value is false. In this case it is
    // safe to use the Labeler but any attributes added to it will not be used.
    func LabelerFromContext(ctx context.Context) (*Labeler, bool) {
    l, ok := ctx.Value(lablelerContextKey).(*Labeler)
    if !ok {
    l = &Labeler{}
    }
    return l, ok
    }
  • make injectLabeler public to be invokable outside otelhttp module

After the changes above will be made the following code sample it will be possible:

client := otelhttp.DefaultClient
labeler, _ := otelhttp.LabelerFromContext(ctx)
labeler.Add(
	AppClientID(123),
	AppCredentialID(456),
)
ctx = otelhttp.InjectLabeler(ctx, labeler)
req, _ := http.NewRequestWithContext(ctx, "GET", *url, nil)
res, err := client.Do(req)
if err != nil {
	log.Error(err)
}
defer res.Body.Close()
@zailic zailic added area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp labels Feb 13, 2024
zailic pushed a commit to zailic/opentelemetry-go-contrib that referenced this issue Feb 13, 2024
zailic added a commit to zailic/opentelemetry-go-contrib that referenced this issue Feb 13, 2024
zailic added a commit to zailic/opentelemetry-go-contrib that referenced this issue Feb 18, 2024
MrAlias added a commit that referenced this issue May 29, 2024
OK... i opened a new PR for #5084 issue, as i shot myself in a foot
trying to modify a previous commit that I pushed under another github
user.

Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@zailic
Copy link
Contributor Author

zailic commented Jul 30, 2024

Fixed in #5129. Closing this issue.

@zailic zailic closed this as completed Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working instrumentation: otelhttp
Projects
None yet
Development

No branches or pull requests

1 participant