-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[exporter/debug] add option to only show certain attributes #13378
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
base: main
Are you sure you want to change the base?
[exporter/debug] add option to only show certain attributes #13378
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13378 +/- ##
==========================================
- Coverage 91.53% 91.51% -0.02%
==========================================
Files 528 529 +1
Lines 29469 29544 +75
==========================================
+ Hits 26974 27038 +64
- Misses 1969 1976 +7
- Partials 526 530 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
exporter/debugexporter/config.go
Outdated
@@ -18,6 +18,10 @@ var supportedLevels map[configtelemetry.Level]struct{} = map[configtelemetry.Lev | |||
configtelemetry.LevelDetailed: {}, | |||
} | |||
|
|||
type OutputConfig struct { | |||
Attributes []string `mapstructure:"attributes"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defines the configuration option output::attributes
as a list of attributes that user wants to see in the exporter's output. If this list is empty, all attributes are displayed in the output. In this solution, is there a way for user to configure the exporter to not display any attributes at all?
Let me propose the following alternate configuration options:
output::attributes::enabled
(boolean, default:true
) when set tofalse
, no attributes are displayedoutput::attributes::include
(list of strings, default empty) list of attributes to display
Please let me know what you think.
exporter/debugexporter/exporter.go
Outdated
@@ -60,6 +71,22 @@ func (s *debugExporter) pushTraces(_ context.Context, td ptrace.Traces) error { | |||
return nil | |||
} | |||
|
|||
for _, rs := range td.ResourceSpans().All() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes the Debug exporter to one that mutates data. This comes with a performance penalty - the whole telemetry batch may need to be copied in memory. I'd rather keep the Debug exporter as non-mutating. Would it be possible to perform the filtering when formatting output?
exporter/debugexporter/exporter.go
Outdated
@@ -60,6 +71,22 @@ func (s *debugExporter) pushTraces(_ context.Context, td ptrace.Traces) error { | |||
return nil | |||
} | |||
|
|||
for _, rs := range td.ResourceSpans().All() { | |||
rs.Resource().Attributes().RemoveIf(func(k string, _ pcommon.Value) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my mind, the output::attributes
only affects the output of record-level attributes (log record attributes, metric data point attributes, span attributes). The output of resource or scope level attributes should be governed by separate options output::resource
and output::scope
.
Perhaps better to name the option output::record::attributes
? What do you think?
@@ -53,7 +53,7 @@ func createDefaultConfig() component.Config { | |||
func createTraces(ctx context.Context, set exporter.Settings, config component.Config) (exporter.Traces, error) { | |||
cfg := config.(*Config) | |||
exporterLogger := createLogger(cfg, set.Logger) | |||
debug := newDebugExporter(exporterLogger, cfg.Verbosity) | |||
debug := newDebugExporter(exporterLogger, cfg.Verbosity, cfg.Output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps just pass the whole config as a single variable?
debug := newDebugExporter(exporterLogger, cfg.Verbosity, cfg.Output) | |
debug := newDebugExporter(exporterLogger, cfg) |
All the comments make sense to me — I'll update accordingly. Regarding filtering when formatting the output, I was thinking of adding a custom Marshaler. Do you think that would be a reasonable approach? |
Thank you @cuichenli for this contribution. I do have a couple comments, please let me know what you think. I've put down a more specific description of the options I have in mind on the issue #9372 (comment). Happy to hear your or others' thoughts on this. |
Feel free to propose a solution, I'll look into it. |
@andrzej-stencel updated, please take another look. thanks! |
Description
Link to tracking issue
Initial implementation for #9372
Testing
Documentation