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

Add a custom label to automatically generated traces #7430

Closed
Gregordy opened this issue Nov 5, 2021 · 8 comments · Fixed by #7676 or #7686
Closed

Add a custom label to automatically generated traces #7430

Gregordy opened this issue Nov 5, 2021 · 8 comments · Fixed by #7676 or #7686
Assignees
Labels
api: cloudtrace Issues related to the Cloud Trace API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@Gregordy
Copy link

Gregordy commented Nov 5, 2021

I am using Google.Cloud.Diagnostics.AspNetCore v4.3.0.

I use AddOutgoingGoogleTraceHandler to trace every outgoing HTTP request of a given HTTP client and I want to add a label to every trace created by this method.

From what I understood, it is more precisely the underlying span of a trace that carries a label.
And we can use the AnnotateSpan method to add a label, but only when we manually create a trace span.

I read the latest documentation but still didn't figure out how to do label annotation for automatically created traces.

Am I missing something?

@jskeet jskeet added api: cloudtrace Issues related to the Cloud Trace API. type: question Request for information or clarification. Not an issue. labels Nov 5, 2021
@amanda-tarafa amanda-tarafa added priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Nov 5, 2021
@amanda-tarafa
Copy link
Contributor

Unfurtunatelly you are not missing anything, custom labels on outgoing traces (and yes, it's the spans that carry the labels) are not supported.
I've marked this as a feature request and I'll try to get it in before the month is out.
Would it work for you a definition of

public interface IOutgoingTraceLabelsProvider
{
    IDictionary<string, string> GetLabelsForOutgoingRequest(HttpRequestMessage outgoingRequest);
}

Then you would inject as many of these as you'd like and we would use them to annotate the outgoing span.

Let me know if this sounds like a good solution for you.

@Gregordy
Copy link
Author

Gregordy commented Nov 5, 2021

Thank you for the quick reply Amanda!

Yes, this solution looks totally fine to me.

amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 11, 2022
@amanda-tarafa
Copy link
Contributor

@Gregordy I've submitted #7676 for review that should add support for custom labels in outgoing traces. I implemented it slightly different to how we talked about in #7430 (comment).

Basically I've added an overload to AddOutgoingGoogleTraceHandler that takes a Func<IServiceProvider, IDictionary<string, string>>. I believe this approach is more inline with how a configuration should work for IHttpClientFactory and it's also more flexible, as it will allow you to configure different HTTP clients that set different labels.

Let me know if you think there's some use case that this won't cover.

amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 11, 2022
amanda-tarafa added a commit that referenced this issue Jan 11, 2022
@amanda-tarafa
Copy link
Contributor

@Gregordy I've merged #7676, but I won't release until you've confirmed that the resulting API surface covers your use case. Please see #7430 (comment) above.

@amanda-tarafa
Copy link
Contributor

@Gregordy any thoughts on the implemented solution?

@Gregordy
Copy link
Author

Gregordy commented Jan 14, 2022

@amanda-tarafa, your implementation is indeed more flexible than what we discussed initially.

Everything looks good to me, I think this will cover all my needs.

@amanda-tarafa
Copy link
Contributor

Thanks! I'll release on Monday then.

@Gregordy
Copy link
Author

Thanks to you Amanda.

amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 17, 2022
Changes in Google.Cloud.Diagnostics.AspNetCore version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnosticsForAspNetCore empty overload. ([commit 18ecd5c](googleapis@18ecd5c))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](googleapis#7633)

Changes in Google.Cloud.Diagnostics.AspNetCore3 version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnosticsForAspNetCore empty overload. ([commit 18ecd5c](googleapis@18ecd5c))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](googleapis#7633)

Changes in Google.Cloud.Diagnostics.Common version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnostics empty overload. ([commit d5c3963](googleapis@d5c3963))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](googleapis#7633)
- Use service version instead of name ([commit 437e6c4](googleapis@437e6c4))

### New features

- Custom labels for outgoing traces. ([commit d8d213c](googleapis@d8d213c))
 - Closes [issue 7430](googleapis#7430)

Packages in this release:
- Release Google.Cloud.Diagnostics.AspNetCore version 4.4.0
- Release Google.Cloud.Diagnostics.AspNetCore3 version 4.4.0
- Release Google.Cloud.Diagnostics.Common version 4.4.0
gcf-merge-on-green bot pushed a commit that referenced this issue Jan 17, 2022
Changes in Google.Cloud.Diagnostics.AspNetCore version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnosticsForAspNetCore empty overload. ([commit 18ecd5c](18ecd5c))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](#7633)

Changes in Google.Cloud.Diagnostics.AspNetCore3 version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnosticsForAspNetCore empty overload. ([commit 18ecd5c](18ecd5c))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](#7633)

Changes in Google.Cloud.Diagnostics.Common version 4.4.0:

### Bug fixes

- Adds AddGoogleDiagnostics empty overload. ([commit d5c3963](d5c3963))
 - To avoid ambiguity, which results in a compiler error, when not specifying any of the optional parameters for the other overloads.
 - Fixes [issue 7633](#7633)
- Use service version instead of name ([commit 437e6c4](437e6c4))

### New features

- Custom labels for outgoing traces. ([commit d8d213c](d8d213c))
 - Closes [issue 7430](#7430)

Packages in this release:
- Release Google.Cloud.Diagnostics.AspNetCore version 4.4.0
- Release Google.Cloud.Diagnostics.AspNetCore3 version 4.4.0
- Release Google.Cloud.Diagnostics.Common version 4.4.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudtrace Issues related to the Cloud Trace API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
3 participants