Log trace association in Google.Cloud.Diagnostics.* 4.2.0-beta01

[EatonZ]

If the load balancer decides not to trace (does not send ;o=1 to my service), the log entry is missing trace, spanId, and traceSampled.

I'm not sure if that is correct, because now the log entry will have no association with the span/trace. Shouldn't trace and spanId still be set, but not traceSampled in these cases?

[amanda-tarafa]

Thanks @EatonZ for using the library so fast and for creating the new issue :).

When the Diagnostics tracing component is configured, if the mask is not set or is set to 0 (;o=0), then we completely ignore the trace/span: we don't set it in entries, we don't set traceSampled to false either and we don't allow for the creation of new spans, etc. That behaviour is from way before traceSampled was available (ignoring the trace was the only way of signaling not sampled) and changing that would be potentially breaking. I explicitly didn't want to do that. I did take note of this so we can consider redesigning the related bits if we release a next major version though.
For the external trace providers I followed this same pattern, that's why the IExternalTraceProvider interface does not have a ShouldTrace or similar property, because if it returns a TraceId, we just assume that the trace is being sampled. i.e. we only ever set traceSampled to true.
Apart from not using the traceSampled field to "its full potential", which I agree with your point, and the fact that you could maybe filter the logs by the TraceId even if the trace is not being sampled, is there something else you see this affecting you or anyone else?
Also, are you certain that the load balancer still sends a tracing header even if it doesn't sample it? Just asking, I don't know.

You can try registering the TraceIdLogEntryLabelProvider. This is not the Google Trace, but an ASP.NET Core generated trace ID that is generated per request. And it's set in a "trace_identifier" label, but you can filter on that.

[EatonZ]

is there something else you see this affecting you or anyone else?

It just seems correct to me to put in the span and trace IDs in logs when possible. Mainly to keep things organized. In some cases there won't be any IDs since the library will not create a span/provide any IDs, but the load balancer provides the IDs.

Also, are you certain that the load balancer still sends a tracing header even if it doesn't sample it? Just asking, I don't know.

It does. Interestingly, it seems to sample randomly and not according to any setting. I already filed a bug for that.

[amanda-tarafa]

It just seems correct to me to put in the span and trace IDs in logs when possible. Mainly to keep things organized. In some cases there won't be any IDs since the library will not create a span/provide any IDs, but the load balancer provides the IDs.

Yes, agree. But as per my explanation, doing so now is a breaking change. I've taken note to possibly include it in the future, no ETA whatsoever though.

To keep formal track of this, I'm now closing this issue and adding it to our backlog in #5392.

[EatonZ]

@amanda-tarafa Ok, thanks. Just to confirm, there's no workaround to force-set those fields in the meantime?

I did note your ASP.NET suggestion.

[amanda-tarafa]

You can use your GoogleTraceProvider instead of the one that I added. Yours didn't take into account the mask to return the TraceId and the SpanId from the header, but, do take into account that traceSampled will be set to true.

Also, I wonder if you force the mask (;o=1) on the response header, if the trace ends up being sampled. You might be able to do that on your GoogleTraceProvider as well, accessing the HttpContext.Response.

But do take into accout that both those things are hacky workarounds, so, if you don't find specific documentation from Google Logging and Tracing that supports these, even if they work now for you, they can stop working at any time.

[EatonZ]

You can use your GoogleTraceProvider instead of the one that I added. Yours didn't take into account the mask to return the TraceId and the SpanId from the header, but, do take into account that traceSampled will be set to true.

Indeed - that's not ideal.

Also, I wonder if you force the mask (;o=1) on the response header, if the trace ends up being sampled.

The load balancer has already made up its mind by that point, and I don't want to send that info to the client (load balancer does not send this header to the client by default).

Hopefully you'll consider this for your next major version. In the meantime, as long as this bug gets fixed, everything should be ok for me.