Add TraceFlags support to logs exporter #690
Conversation
| if traceSampled, ok := attrsMap[TraceSampledAttributeKey]; ok { | ||
| entry.TraceSampled = traceSampled.Bool() | ||
| // parse TraceSampled boolean from OTel attribute or IsSampled OTLP flag | ||
| if traceSampled, ok := attrsMap[TraceSampledAttributeKey]; ok || logRecord.Flags().IsSampled() { |
There was a problem hiding this comment.
Just curious, why did we have a "special" attribute in the first place?
There was a problem hiding this comment.
Should we update the spec to say that we should use the log record sampled flag instead of this attribute? We can keep it in the exporter for backwards-compatibility, but sampled isn't something I expect to get from attributes generally speaking.
There was a problem hiding this comment.
Yup good point, either way if we're adding support for log Flags we should update that spec doc to show it. I'll open a PR for that too
There was a problem hiding this comment.
Codecov Report
@@ Coverage Diff @@
## main #690 +/- ##
==========================================
- Coverage 69.31% 69.22% -0.09%
==========================================
Files 36 36
Lines 4728 4728
==========================================
- Hits 3277 3273 -4
- Misses 1297 1301 +4
Partials 154 154
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Fixes #685
Adds support for TraceFlags to parse traceID sampling (alongside the existing
gcp.trace_sampledattribute)Right now, if either flag is set then TraceSampled will be true. We might want to shift to one taking precedence, but for backward compatibility this is the least breaking way to introduce the feature.