-
Notifications
You must be signed in to change notification settings - Fork 371
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
Establish a trace context in a non-http context. #5897
Comments
Tracing itself isn't dependent on an HTTP context. Our ASP.NET Core part is, for natural reasons. I suspect that the real answer is going to be integration with OpenTelemetry - to be honest, I hope that we can provide a good logging format to stdout that will avoid needing a logging API, and OpenTelemetry integration for tracing, allowing us to avoid a "heavy" integration at all. Amanda may well have more information. |
@jskeet I agree that OpenTelemetry is probably a better fit, but I'm having a hard time figuring out where we're at with that. We're currently settled on the W3C trace context format and a few blogs suggest Google has (or is) implementing it and it is already available in some areas. I'm trying to find a solution that allows us to trace and log using W3C Trace Context to keep them linked. I agree that I'd rather just emit this stuff to stdout. I guess the greatest challenge right now is establish the trace context outside of an HTTP context. We do have a support contract btw (we're a well known enterprise migrating from Azure to GCP, just now kicking off), and this is a bit of a sticking point. I wanted to gauge interest here first and I can open it inside of cloud console. |
I suspect you can do everything you need already with what's in the ASP.NET Core Diagnostics package, or with the "raw" tracing/logging APIs (which can certainly be used directly). Amanda may have more information. We haven't been prioritizing OpenTelemetry so far, partly as it's still in beta (and we're a small team), but I would hope we'll set aside some time at some point. (I appreciate that doesn't help you right now.) |
@jskeet Oh that's good information. This is a huge talking point with us. We were under the impression that we wouldn't have to use proprietary cloud trace headers as we build out other systems that are not hosted in GCP. We have a team that's trying to refine some logging in other non-cloud systems but pipe them through cloud trace, using that w3c format so we can trace full transactions across cloud and non cloud systems if that makes sense. If we still have to use X-Cloud-Trace-Context while OpenTelemetry integration is being developed, that would be good to know -- we could still continue as planned but utilize the tracestate to keep the cloud trace headers around. Am I understanding this right? OpenTelemetry would be a different package that still sinks to stackdriver/cloud trace? Or is it something else completely? Is there a roadmap somewhere by chance? |
I would expect OpenTelemetry integration to be via a different package, yes. (I suspect there'd be a package which is just a Cloud Trace sink, and relies on OpenTelemetry itself for all the integration.) We haven't had the time to look deeply at OpenTelemetry yet. No roadmap for all this I'm afraid, more due to being busy with other work than anything else. If you could give some more information about your scenario (are you using ASP.NET Core at all, or should we be talking about other contexts?) I suspect @amanda-tarafa will be able to help you more concretely than I can. |
@jstafford5380 I'll try to answer/comment on all your points, but please come back if I've missed something: On OpenTelemetry: What have stopped us from even starting to look at OpenTelemetry integration is basically this notice on the .NET OpenTelemetry repo. It's not just that the library is in beta, but also that they don't guarantee to follow a specific version of the spec and are making emphasis on not even attempting to be backwards compatible from release to release. That means we'd be playing catch right now, and as @jskeet said we are a small team. On the W3C format: Some of GCP's infrastructure already supports On using other than On Activity (I think you mean Let me know. |
@amanda-tarafa Sorry for the delay, I missed the response email... I see and that's a little disheartening to hear, for sure. On Ideally, what I would want to see is that establishing a trace when there is no http context should be the same as when there is an http context. That is, once it's established, the rest of the pipeline, so to speak, will continue on as normal. This is of course to avoid any sort of "modal setup" that developers would be expected to obey. I'd hate to have to develop a message handler or service in code that has to check whether or not it's processing a request in an HTTP context or a non-HTTP context so that it knows whether or not it needs to insert a custom HTTP handler before making an outbound request, just to ensure a trace is continued, if that makes sense. |
Yes, If I'm understanding you correctly, you want an Activity based layer that can be initialized from multiple sources, one of them being HTTP, one of them being Pub/Sub subscribers, maybe some others. But, I think you can use
This might seem like a little too much, but is really not. Notice that you don't even need to write the Activity based layer if you are OK with using If I didn't understand right, I'd need a little more context, maybe some code snippets of what you want to achieve to try and help you better. Let me know. |
No, not at all! I think that makes sense. I think I would need to figure out how to create get traceid, ulong, spanid from a trace traceparent though in order to create that context. |
Microsoft have created System.DiagnosticSource.Activity that allows you to capture TraceId and SpanId. |
BTW, OpenTelemetry .NET reaches v1.0 https://devblogs.microsoft.com/dotnet/opentelemetry-net-reaches-v1-0/ |
Thanks @StasPerekrestov. Yes we are already looking into next steps. An annoucement will be made once we have a plan. |
@jstafford5380 Given your latest comments on #5929 I was wondering if you still have the need for this feature or not, i.e. the hook to provide trace information that is not comming from Google's own trace header. @StasPerekrestov you've also shown interest. Is my proposed approach in #5897 (comment) something that would work for you? Would you need the hook as well for obaining the trace information from other than Google's own trace header? I'm just prioritizing the Diagnostics work and this info will help me do that. Thanks. |
Yeah, I think it's still needed. We still need the ability to re-establish
a trace context that didn't necessarily come in from an http request.
…On Tue, Apr 6, 2021 at 8:45 AM Amanda Tarafa Mas ***@***.***> wrote:
@jstafford5380 <https://github.com/jstafford5380> Given your latest
comments on #5929
<#5929> I was
wondering if you still have the need for this feature or not, i.e. the hook
to provide trace information that is not comming from Google's own trace
header.
@StasPerekrestov <https://github.com/StasPerekrestov> you've also shown
interest. Is my proposed approach in #5897 (comment)
<#5897 (comment)>
something that would work for you? Would you need the hook as well for
obaining the trace information from other than Google's own trace header?
I'm just prioritizing the Diagnostics work and this info will help me do
that. Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5897 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALI4REDEUCWV7Q7WVPDPIELTHMUDDANCNFSM4W5OJ3JQ>
.
|
In my case, I use a subscription to pubnub and there is no HttpContext when I receive a message. |
OK, thanks for the info. I'll work on this then. |
I've just merged #6568 that addresses this issue fully by:
I'll flag here when we've released, I first want to addres a couple more requests that we have open. |
Changes in Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01: - [Commit 60e8cd8](googleapis@60e8cd8): - feat: Copies GoogleLogger to Common. This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](googleapis#6367) - Replicate LoggerOptions in Common, and have AspNetCore\*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common and marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. It can now be replaced by Common.ITraceContext. - [Commit 32cb606](googleapis@32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](googleapis#5313) - Closes [issue 5929](googleapis#5929) - [Commit c8e9a48](googleapis@c8e9a48): - refactor: Makes GoogleLoggerScope extendable. Moves GoogleLoggerScope to Diagnostics.Common. In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. Towards [issue 5313](googleapis#5313), [issue 5360](googleapis#5360), [issue 5929](googleapis#5929) and [issue 6367](googleapis#6367) - [Commit 7f5f89e](googleapis@7f5f89e): - docs: Change Stackdriver to Google Cloud, and fix some typos, including in test code. - [Commit c4c9cd5](googleapis@c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications Closes [issue 5897](googleapis#5897) Towards [issue 6367](googleapis#6367) - [Commit b35b9ea](googleapis@b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header. Towards [issue 5360](googleapis#5360) and [issue 5897](googleapis#5897) - [Commit 0c00d2c](googleapis@0c00d2c): - refactor: Remove unnecesary service provider extension method. There's an equivalent method offered by Microsoft.Extensions.DependencyInjection so we don't need our own. - [Commit bb0c7b2](googleapis@bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](googleapis@8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. See https://cloud.google.com/trace/docs/setup#force-trace Note: changing a generic type parameter constraint in `LabelProviderExtensions` is notionally a breaking change, but due to how it will be used, we don't expect it to actually break any customers. Changes in Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01: - [Commit 60e8cd8](googleapis@60e8cd8): - feat: Copies GoogleLogger to Common. - This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](googleapis#6367) - Replicate LoggerOptions in Common. - And have AspNetCore*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common. - And marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. - It can now be replaced by Common.ITraceContext. - [Commit 32cb606](googleapis@32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](googleapis#5313) - Closes [issue 5929](googleapis#5929) - [Commit c8e9a48](googleapis@c8e9a48): - refactor: Makes GoogleLoggerScope extendable. - Moves GoogleLoggerScope to Diagnostics.Common. - In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. - Towards [issue 5313](googleapis#5313), [issue 5360](googleapis#5360), [issue 5929](googleapis#5929) and [issue 6367](googleapis#6367) - [Commit 7f5f89e](googleapis@7f5f89e): - docs: Change Stackdriver to Google Cloud - And fix some typos, including in test code. - [Commit c4c9cd5](googleapis@c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications - Closes [issue 5897](googleapis#5897) - Towards [issue 6367](googleapis#6367) - [Commit b35b9ea](googleapis@b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header - Towards [issue 5360](googleapis#5360) and [issue 5897](googleapis#5897) - [Commit bb0c7b2](googleapis@bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. - It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](googleapis@8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. - See https://cloud.google.com/trace/docs/setup#force-trace Packages in this release: - Release Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.Common version 4.3.0-beta01
Changes in Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01: - [Commit 60e8cd8](googleapis@60e8cd8): - feat: Copies GoogleLogger to Common. This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](googleapis#6367) - Replicate LoggerOptions in Common, and have AspNetCore\*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common and marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. It can now be replaced by Common.ITraceContext. - [Commit 32cb606](googleapis@32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](googleapis#5313) - Closes [issue 5929](googleapis#5929) - [Commit c8e9a48](googleapis@c8e9a48): - refactor: Makes GoogleLoggerScope extendable. Moves GoogleLoggerScope to Diagnostics.Common. In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. Towards [issue 5313](googleapis#5313), [issue 5360](googleapis#5360), [issue 5929](googleapis#5929) and [issue 6367](googleapis#6367) - [Commit 7f5f89e](googleapis@7f5f89e): - docs: Change Stackdriver to Google Cloud, and fix some typos, including in test code. - [Commit c4c9cd5](googleapis@c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications Closes [issue 5897](googleapis#5897) Towards [issue 6367](googleapis#6367) - [Commit b35b9ea](googleapis@b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header. Towards [issue 5360](googleapis#5360) and [issue 5897](googleapis#5897) - [Commit 0c00d2c](googleapis@0c00d2c): - refactor: Remove unnecesary service provider extension method. There's an equivalent method offered by Microsoft.Extensions.DependencyInjection so we don't need our own. - [Commit bb0c7b2](googleapis@bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](googleapis@8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. See https://cloud.google.com/trace/docs/setup#force-trace Note: changing a generic type parameter constraint in `LabelProviderExtensions` is notionally a breaking change, but due to how it will be used, we don't expect it to actually break any customers. Changes in Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01: - [Commit 60e8cd8](googleapis@60e8cd8): - feat: Copies GoogleLogger to Common. This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](googleapis#6367) - Replicate LoggerOptions in Common, and have AspNetCore\*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common and marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. It can now be replaced by Common.ITraceContext. - [Commit 32cb606](googleapis@32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](googleapis#5313) - Closes [issue 5929](googleapis#5929) - [Commit c8e9a48](googleapis@c8e9a48): - refactor: Makes GoogleLoggerScope extendable. Moves GoogleLoggerScope to Diagnostics.Common. In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. Towards [issue 5313](googleapis#5313), [issue 5360](googleapis#5360), [issue 5929](googleapis#5929) and [issue 6367](googleapis#6367) - [Commit 7f5f89e](googleapis@7f5f89e): - docs: Change Stackdriver to Google Cloud, and fix some typos, including in test code. - [Commit c4c9cd5](googleapis@c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications Closes [issue 5897](googleapis#5897) Towards [issue 6367](googleapis#6367) - [Commit b35b9ea](googleapis@b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header. Towards [issue 5360](googleapis#5360) and [issue 5897](googleapis#5897) - [Commit 0c00d2c](googleapis@0c00d2c): - refactor: Remove unnecesary service provider extension method. There's an equivalent method offered by Microsoft.Extensions.DependencyInjection so we don't need our own. - [Commit bb0c7b2](googleapis@bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](googleapis@8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. See https://cloud.google.com/trace/docs/setup#force-trace Note: changing a generic type parameter constraint in `LabelProviderExtensions` is notionally a breaking change, but due to how it will be used, we don't expect it to actually break any customers. Packages in this release: - Release Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.Common version 4.3.0-beta01
Changes in Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01: - [Commit 60e8cd8](60e8cd8): - feat: Copies GoogleLogger to Common. This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](#6367) - Replicate LoggerOptions in Common, and have AspNetCore\*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common and marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. It can now be replaced by Common.ITraceContext. - [Commit 32cb606](32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](#5313) - Closes [issue 5929](#5929) - [Commit c8e9a48](c8e9a48): - refactor: Makes GoogleLoggerScope extendable. Moves GoogleLoggerScope to Diagnostics.Common. In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. Towards [issue 5313](#5313), [issue 5360](#5360), [issue 5929](#5929) and [issue 6367](#6367) - [Commit 7f5f89e](7f5f89e): - docs: Change Stackdriver to Google Cloud, and fix some typos, including in test code. - [Commit c4c9cd5](c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications Closes [issue 5897](#5897) Towards [issue 6367](#6367) - [Commit b35b9ea](b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header. Towards [issue 5360](#5360) and [issue 5897](#5897) - [Commit 0c00d2c](0c00d2c): - refactor: Remove unnecesary service provider extension method. There's an equivalent method offered by Microsoft.Extensions.DependencyInjection so we don't need our own. - [Commit bb0c7b2](bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. See https://cloud.google.com/trace/docs/setup#force-trace Note: changing a generic type parameter constraint in `LabelProviderExtensions` is notionally a breaking change, but due to how it will be used, we don't expect it to actually break any customers. Changes in Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01: - [Commit 60e8cd8](60e8cd8): - feat: Copies GoogleLogger to Common. This allows easier use of GoogleLogger in non ASP.NET Core applications. - Towards [issue 6367](#6367) - Replicate LoggerOptions in Common, and have AspNetCore\*.LoggerOptions be just a wrapper of Common.LoggerOptions. - Copies ILogEntryLabelProvider to Common and marks the one in AspNetCore* Obsolete. - Makes AspNetCore*.IExternalTraceProvider obsolete. It can now be replaced by Common.ITraceContext. - [Commit 32cb606](32cb606): - feat: Allows per log entry labels. - Closes [issue 5313](#5313) - Closes [issue 5929](#5929) - [Commit c8e9a48](c8e9a48): - refactor: Makes GoogleLoggerScope extendable. Moves GoogleLoggerScope to Diagnostics.Common. In preparation for allowing LogEntry augmentation and making it easier to use Google logging from non ASP.NET Core apps. Towards [issue 5313](#5313), [issue 5360](#5360), [issue 5929](#5929) and [issue 6367](#6367) - [Commit 7f5f89e](7f5f89e): - docs: Change Stackdriver to Google Cloud, and fix some typos, including in test code. - [Commit c4c9cd5](c4c9cd5): - feat: Makes it easier to use tracing from non ASP.NET Core applications Closes [issue 5897](#5897) Towards [issue 6367](#6367) - [Commit b35b9ea](b35b9ea): - feat: Decouples Diagnostics tracing from Google's trace header. Towards [issue 5360](#5360) and [issue 5897](#5897) - [Commit 0c00d2c](0c00d2c): - refactor: Remove unnecesary service provider extension method. There's an equivalent method offered by Microsoft.Extensions.DependencyInjection so we don't need our own. - [Commit bb0c7b2](bb0c7b2): - refactor: Remove unnecesary interface IManagedTracerFactory. It's an internal interface that we don't use anywhere. We can always add it back in if we need it at some point. - [Commit 8ef3983](8ef3983): - fix: X-Cloud-Trace-Context trace mask values should be 0-1. See https://cloud.google.com/trace/docs/setup#force-trace Note: changing a generic type parameter constraint in `LabelProviderExtensions` is notionally a breaking change, but due to how it will be used, we don't expect it to actually break any customers. Packages in this release: - Release Google.Cloud.Diagnostics.AspNetCore version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.AspNetCore3 version 4.3.0-beta01 - Release Google.Cloud.Diagnostics.Common version 4.3.0-beta01
Is your feature request related to a problem? Please describe.
Tracing seems dependent on an HTTP Context. When I am performing a function in response to a message coming in off of pubsub for example, I cannot establish an http context for the "request" nor would I want to, however the trace information may be available on the bus message so I should be able to re-establish the trace context and continue it. This results in broken traces once a transaction/span hits a service bus of any kind.
Describe the solution you'd like
I think ideally, the
Activity
object would be in place of (or in tandem with) theTraceHeaderContext
. Activities are created whenever an entrypoint is engaged. For example when an HTTP request begins (incoming) or when the message from the bus is dispatched to a handler.As luck would have it, the Activity also supports the W3C trace context format, making it easy to establish that. Since it would appear that this SDK doesn't yet support that either, we can use the tracestate to propogate the X-Cloud-Trace header and its value. Possibly knock out two enhancements in one. An adapter may be warranted.
Describe alternatives you've considered
I've considered creating a different scoped service. Sort of a property bag that would be accessible through DI but this wouldn't be consistent with how traces are establish when we actually DO have an HTTP context. It would be better to have a unified method, which I think lies in the
Activity
.The text was updated successfully, but these errors were encountered: