Skip to content

Commit

Permalink
[exporter/splunkhec] Fix Capabilities with enabled batching (open-tel…
Browse files Browse the repository at this point in the history
…emetry#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 <[email protected]>
  • Loading branch information
dmitryax and atoulme committed Sep 20, 2024
1 parent 1487e43 commit ed2610f
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 7 deletions.
28 changes: 28 additions & 0 deletions .chloggen/splunkhec-fix-invalid-data-access.yaml
Original file line number Diff line number Diff line change
@@ -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: []
3 changes: 2 additions & 1 deletion exporter/splunkhecexporter/batchperscope.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions exporter/splunkhecexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
12 changes: 6 additions & 6 deletions pkg/batchperresourceattr/batchperresourceattr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit ed2610f

Please sign in to comment.