From ed2610fdff9ad95e2f94e9998ff1f7ab94354259 Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Fri, 20 Sep 2024 09:06:00 -0700 Subject: [PATCH] [exporter/splunkhec] Fix Capabilities with enabled batching (#35306) The exporter wrappers, like batch by scope and batch by resource, incorrectly claim that they are not mutating even if the wrapped exporter mutates the data. This leads to "invalid data access" runtime failures. This change fixes that. Co-authored-by: Antoine Toulme --- .../splunkhec-fix-invalid-data-access.yaml | 28 ++++++++++++++++ exporter/splunkhecexporter/batchperscope.go | 3 +- exporter/splunkhecexporter/factory_test.go | 32 +++++++++++++++++++ .../batchperresourceattr.go | 12 +++---- 4 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 .chloggen/splunkhec-fix-invalid-data-access.yaml diff --git a/.chloggen/splunkhec-fix-invalid-data-access.yaml b/.chloggen/splunkhec-fix-invalid-data-access.yaml new file mode 100644 index 000000000000..4eeb3b235915 --- /dev/null +++ b/.chloggen/splunkhec-fix-invalid-data-access.yaml @@ -0,0 +1,28 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: exporter/splunkhec + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix incorrect claim that the exporter doesn't mutate data when batching is enabled. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [35306] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + The bug lead to runtime panics when the exporter was used with the batcher enabled in a fanout scenario. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/exporter/splunkhecexporter/batchperscope.go b/exporter/splunkhecexporter/batchperscope.go index 0c0c51361a7a..388ad525f84a 100644 --- a/exporter/splunkhecexporter/batchperscope.go +++ b/exporter/splunkhecexporter/batchperscope.go @@ -20,8 +20,9 @@ type perScopeBatcher struct { next consumer.Logs } +// Capabilities returns capabilities of the next consumer because perScopeBatcher doesn't mutate data itself. func (rb *perScopeBatcher) Capabilities() consumer.Capabilities { - return consumer.Capabilities{MutatesData: false} + return rb.next.Capabilities() } func (rb *perScopeBatcher) ConsumeLogs(ctx context.Context, logs plog.Logs) error { diff --git a/exporter/splunkhecexporter/factory_test.go b/exporter/splunkhecexporter/factory_test.go index 3a7f7d7f9328..b3349d74225d 100644 --- a/exporter/splunkhecexporter/factory_test.go +++ b/exporter/splunkhecexporter/factory_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component/componenttest" "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/exporter/exporterbatcher" "go.opentelemetry.io/collector/exporter/exportertest" ) @@ -88,3 +89,34 @@ func TestFactory_CreateMetricsExporter(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, te) } + +func TestFactory_EnabledBatchingMakesExporterMutable(t *testing.T) { + config := &Config{ + Token: "testToken", + ClientConfig: confighttp.ClientConfig{ + Endpoint: "https://example.com:8000", + }, + } + + me, err := createMetricsExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.False(t, me.Capabilities().MutatesData) + te, err := createTracesExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.False(t, te.Capabilities().MutatesData) + le, err := createLogsExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.False(t, le.Capabilities().MutatesData) + + config.BatcherConfig = exporterbatcher.NewDefaultConfig() + + me, err = createMetricsExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.True(t, me.Capabilities().MutatesData) + te, err = createTracesExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.True(t, te.Capabilities().MutatesData) + le, err = createLogsExporter(context.Background(), exportertest.NewNopSettings(), config) + require.NoError(t, err) + assert.True(t, le.Capabilities().MutatesData) +} diff --git a/pkg/batchperresourceattr/batchperresourceattr.go b/pkg/batchperresourceattr/batchperresourceattr.go index 92f55b6481da..510b41f6fca2 100644 --- a/pkg/batchperresourceattr/batchperresourceattr.go +++ b/pkg/batchperresourceattr/batchperresourceattr.go @@ -35,9 +35,9 @@ func NewMultiBatchPerResourceTraces(attrKeys []string, next consumer.Traces) con } } -// Capabilities implements the consumer interface. +// Capabilities returns the capabilities of the next consumer because batchTraces doesn't mutate data itself. func (bt *batchTraces) Capabilities() consumer.Capabilities { - return consumer.Capabilities{MutatesData: false} + return bt.next.Capabilities() } func (bt *batchTraces) ConsumeTraces(ctx context.Context, td ptrace.Traces) error { @@ -98,9 +98,9 @@ func NewMultiBatchPerResourceMetrics(attrKeys []string, next consumer.Metrics) c } } -// Capabilities implements the consumer interface. +// Capabilities returns the capabilities of the next consumer because batchMetrics doesn't mutate data itself. func (bt *batchMetrics) Capabilities() consumer.Capabilities { - return consumer.Capabilities{MutatesData: false} + return bt.next.Capabilities() } func (bt *batchMetrics) ConsumeMetrics(ctx context.Context, td pmetric.Metrics) error { @@ -159,9 +159,9 @@ func NewMultiBatchPerResourceLogs(attrKeys []string, next consumer.Logs) consume } } -// Capabilities implements the consumer interface. +// Capabilities returns the capabilities of the next consumer because batchLogs doesn't mutate data itself. func (bt *batchLogs) Capabilities() consumer.Capabilities { - return consumer.Capabilities{MutatesData: false} + return bt.next.Capabilities() } func (bt *batchLogs) ConsumeLogs(ctx context.Context, td plog.Logs) error {