[elasticsearchexporter] support dynamic mapping modes #38114
Conversation
axw
changed the title
axw
force-pushed
the
mappingmode-context
branch
3 times, most recently
from
f03e17b to
9b5fd32
Compare
Add support for controlling the mapping mode via a client metadata key (typically an HTTP or gRPC header), X-Elastic-Mapping-Mode. This will override the mode specified in `mapping::mode`, which now serves as the default. We also introduce `mapping::allowed_modes` config which restricts the mapping modes to those listed. This will allow an administrator to lock down the allowed mapping modes that can be specified through client metadata. Because we want to set "require_data_stream=true" for otel mode, and we no longer know ahead of time which mapping mode will be used, we now create a BulkIndexer per mapping mode.
axw
force-pushed
the
mappingmode-context
branch
from
9b5fd32 to
2ef0941
Compare
There was a problem hiding this comment.
thanks! I just realize this may not work with sending_queue::enabled: true as client metadata isn't persisted (in memory or on disk). Do we have a plan how to make it work? May not be a blocker if it is consistent with how other parts of es exporter do not work with client metadata. But as we're adding the feature to README, we should note that it is broken if sending queue is enabled.
e.g. Tests fail (not very clear as mapping mode is not asserted) with this change
diff --git a/exporter/elasticsearchexporter/exporter_test.go b/exporter/elasticsearchexporter/exporter_test.go
index 1795f2898c..0c52f157a6 100644
--- a/exporter/elasticsearchexporter/exporter_test.go
+++ b/exporter/elasticsearchexporter/exporter_test.go
@@ -2214,6 +2214,7 @@ func newUnstartedTestLogsExporter(t *testing.T, url string, fns ...func(*Config)
cfg.Endpoints = []string{url}
cfg.NumWorkers = 1
cfg.Flush.Interval = 10 * time.Millisecond
+ cfg.QueueSettings.Enabled = true
}}, fns...)...)
exp, err := f.CreateLogs(context.Background(), exportertest.NewNopSettings(metadata.Type), cfg)
require.NoError(t, err)| rec := newBulkRecorder() | ||
| server := newESTestServer(t, func(docs []itemRequest) ([]itemResponse, error) { | ||
| rec.Record(docs) | ||
| return itemsAllOK(docs) |
There was a problem hiding this comment.
is there a way to quickly check if the supplied mapping mode is respected?
There was a problem hiding this comment.
That's what the check function is doing -- it's verifying the document structure is as expected for the given mapping mode.
Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
IMO that's a bug in |
I looked into it and realize the test failed because the error didn't propagate up the chain. It is not because client metadata isn't propagated in context. Although I wonder if client context is persisted for persistent queue after restart. Let's get to the bottom of it soon, but it is not a blocker.
😢 I missed it as I was incorrectly assuming that bodymap test failed because client metadata isn't persisted |
Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
| mappingMode := e.config.MappingMode() | ||
| mappingMode, err := e.getMappingMode(ctx) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
Shouldn't this be a permanent error?
| return err | |
| return consumererror.NewPermanent(err) |
By the way, it looks like the Elasticsearch exporter does not use consumererror.NewPermanent() anywhere. Does this mean it never returns permanent errors to its callers? Perhaps something worth looking at?
There was a problem hiding this comment.
On a second thought, this is probably not a blocker for this PR, but rather a wider issue to look into, as the Elasticsearch exporter does not seem to make use of permanent errors anywhere.
There was a problem hiding this comment.
Thanks @andrzej-stencel, agreed that we should look into that, and that it's a broader issue.
Description
Add support for controlling the mapping mode via a client metadata key (typically an HTTP or gRPC header), X-Elastic-Mapping-Mode. This will override the mode specified in
mapping::mode, which now serves as the default.We also introduce
mapping::allowed_modesconfig which restricts mapping modes to those listed. This will allow an administrator to lock down the allowed mapping modes that can be specified through client metadata.Because we want to set "require_data_stream=true" for otel mode, and we no longer know ahead of time which mapping mode will be used, we now create a BulkIndexer per mapping mode.
Link to tracking issue
Closes #36092
Testing
--otlp-header X-Elastic-Mapping-Mode=\"<mode>\"Documentation
Updated README.