Skip to content

Commit

Permalink
[elasticsearchexporter] Remove dedot config (open-telemetry#37736)
Browse files Browse the repository at this point in the history
#### Description

Remove the "dedot" config from the Elasticsearch exporter. This config
has been deprecated for over 6 months, so it's time to remove it. ECS
mode now dedots by default, and none of the others dedot at all.

#### Link to tracking issue

Closes
open-telemetry#33772

#### Testing

I only updated unit tests. No other testing performed, since this is a
purely subtractive change that is covered by unit tests.

#### Documentation

Updated README
  • Loading branch information
axw authored Feb 13, 2025
1 parent 9987999 commit ee5078a
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 183 deletions.
27 changes: 27 additions & 0 deletions .chloggen/elasticsearchexporter-rm-dedot-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: elasticsearchexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Remove dedot config. ECS mode now always dedots, no others dedot at all.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [33772]

# (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:

# 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: [user]
3 changes: 0 additions & 3 deletions exporter/elasticsearchexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,6 @@ behaviours, which may be configured through the following settings:
It works only for logs where the log record body is a map. Each LogRecord
body is serialized to JSON as-is and becomes a separate document for ingestion.
If the log record body is not a map, the exporter will log a warning and drop the log record.
- `dedot` (default=true; DEPRECATED, in future dedotting will always be enabled
for ECS mode, and never for other modes): When enabled attributes with `.`
will be split into proper json objects.

#### ECS mapping mode

Expand Down
8 changes: 0 additions & 8 deletions exporter/elasticsearchexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,6 @@ type RetrySettings struct {
type MappingsSettings struct {
// Mode configures the field mappings.
Mode string `mapstructure:"mode"`

// Deprecated: [v0.104.0] dedotting will always be applied for ECS mode
// in future, and never for other modes. Elasticsearch's "dot_expander"
// Ingest processor may be used as an alternative for non-ECS modes.
Dedot bool `mapstructure:"dedot"`
}

type MappingMode int
Expand Down Expand Up @@ -363,9 +358,6 @@ func (cfg *Config) MappingMode() MappingMode {
}

func handleDeprecatedConfig(cfg *Config, logger *zap.Logger) {
if cfg.Mapping.Dedot && cfg.MappingMode() != MappingECS || !cfg.Mapping.Dedot && cfg.MappingMode() == MappingECS {
logger.Warn("dedot has been deprecated: in the future, dedotting will always be performed in ECS mode only")
}
if cfg.Retry.MaxRequests != 0 {
cfg.Retry.MaxRetries = cfg.Retry.MaxRequests - 1
// Do not set cfg.Retry.Enabled = false if cfg.Retry.MaxRequest = 1 to avoid breaking change on behavior
Expand Down
9 changes: 3 additions & 6 deletions exporter/elasticsearchexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ func TestConfig(t *testing.T) {
RetryOnStatus: []int{http.StatusTooManyRequests, http.StatusInternalServerError},
},
Mapping: MappingsSettings{
Mode: "none",
Dedot: true,
Mode: "none",
},
LogstashFormat: LogstashFormatSettings{
Enabled: false,
Expand Down Expand Up @@ -181,8 +180,7 @@ func TestConfig(t *testing.T) {
RetryOnStatus: []int{http.StatusTooManyRequests, http.StatusInternalServerError},
},
Mapping: MappingsSettings{
Mode: "none",
Dedot: true,
Mode: "none",
},
LogstashFormat: LogstashFormatSettings{
Enabled: false,
Expand Down Expand Up @@ -255,8 +253,7 @@ func TestConfig(t *testing.T) {
RetryOnStatus: []int{http.StatusTooManyRequests, http.StatusInternalServerError},
},
Mapping: MappingsSettings{
Mode: "none",
Dedot: true,
Mode: "none",
},
LogstashFormat: LogstashFormatSettings{
Enabled: false,
Expand Down
3 changes: 1 addition & 2 deletions exporter/elasticsearchexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ func newExporter(
dynamicIndex bool,
) *elasticsearchExporter {
model := &encodeModel{
dedot: cfg.Mapping.Dedot,
mode: cfg.MappingMode(),
mode: cfg.MappingMode(),
}

otel := model.mode == MappingOTel
Expand Down
6 changes: 4 additions & 2 deletions exporter/elasticsearchexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ func TestExporterLogs(t *testing.T) {

exporter := newTestLogsExporter(t, server.URL, func(cfg *Config) {
cfg.Mapping.Mode = "ecs"
cfg.Mapping.Dedot = true
// deduplication is always performed except in otel mapping mode -
// there is no other configuration that controls it
})
logs := newLogsWithAttributes(
map[string]any{"attr.key": "value"},
Expand All @@ -235,7 +236,7 @@ func TestExporterLogs(t *testing.T) {
t.Run("publish with dedup", func(t *testing.T) {
rec := newBulkRecorder()
server := newESTestServer(t, func(docs []itemRequest) ([]itemResponse, error) {
assert.JSONEq(t, `{"@timestamp":"1970-01-01T00:00:00.000000000Z","Scope":{"name":"","value":"value","version":""},"SeverityNumber":0,"TraceFlags":0}`, string(docs[0].Document))
assert.JSONEq(t, `{"@timestamp":"1970-01-01T00:00:00.000000000Z","Scope.name":"","Scope.value":"value","Scope.version":"","SeverityNumber":0,"TraceFlags":0}`, string(docs[0].Document))
rec.Record(docs)
return itemsAllOK(docs)
})
Expand Down Expand Up @@ -666,6 +667,7 @@ func TestExporterLogs(t *testing.T) {
})

exporter := newTestLogsExporter(t, server.URL, func(cfg *Config) {
cfg.Mapping.Mode = "otel"
cfg.Flush.Interval = 50 * time.Millisecond
cfg.Retry.InitialInterval = 1 * time.Millisecond
cfg.Retry.MaxInterval = 10 * time.Millisecond
Expand Down
3 changes: 1 addition & 2 deletions exporter/elasticsearchexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ func createDefaultConfig() component.Config {
},
},
Mapping: MappingsSettings{
Mode: "none",
Dedot: true,
Mode: "none",
},
LogstashFormat: LogstashFormatSettings{
Enabled: false,
Expand Down
41 changes: 0 additions & 41 deletions exporter/elasticsearchexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/exporter/exportertest"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
)

func TestCreateDefaultConfig(t *testing.T) {
Expand Down Expand Up @@ -60,42 +58,3 @@ func TestFactory_CreateTraces(t *testing.T) {

require.NoError(t, exporter.Shutdown(context.Background()))
}

func TestFactory_DedotDeprecated(t *testing.T) {
loggerCore, logObserver := observer.New(zap.WarnLevel)
set := exportertest.NewNopSettings()
set.Logger = zap.New(loggerCore)

cfgNoDedotECS := withDefaultConfig(func(cfg *Config) {
cfg.Endpoint = "http://testing.invalid:9200"
cfg.Mapping.Dedot = false
cfg.Mapping.Mode = "ecs"
})

cfgDedotRaw := withDefaultConfig(func(cfg *Config) {
cfg.Endpoint = "http://testing.invalid:9200"
cfg.Mapping.Dedot = true
cfg.Mapping.Mode = "raw"
})

for _, cfg := range []*Config{cfgNoDedotECS, cfgDedotRaw} {
factory := NewFactory()
logsExporter, err := factory.CreateLogs(context.Background(), set, cfg)
require.NoError(t, err)
require.NoError(t, logsExporter.Shutdown(context.Background()))

tracesExporter, err := factory.CreateTraces(context.Background(), set, cfg)
require.NoError(t, err)
require.NoError(t, tracesExporter.Shutdown(context.Background()))

metricsExporter, err := factory.CreateMetrics(context.Background(), set, cfg)
require.NoError(t, err)
require.NoError(t, metricsExporter.Shutdown(context.Background()))
}

records := logObserver.AllUntimed()
assert.Len(t, records, 6)
for _, record := range records {
assert.Equal(t, "dedot has been deprecated: in the future, dedotting will always be performed in ECS mode only", record.Message)
}
}
7 changes: 4 additions & 3 deletions exporter/elasticsearchexporter/internal/objmodel/objmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,11 @@ func newJSONVisitor(w io.Writer) *json.Visitor {
return v
}

// Serialize writes the document to the given writer. The serializer will create nested objects if dedot is true.
//
// NOTE: The documented MUST be sorted if dedot is true.
// Serialize writes the document to the given writer. The document fields will be
// deduplicated and, if dedot is true, turned into nested objects prior to
// serialization.
func (doc *Document) Serialize(w io.Writer, dedot bool) error {
doc.Dedup()
v := newJSONVisitor(w)
return doc.iterJSON(v, dedot)
}
Expand Down
26 changes: 4 additions & 22 deletions exporter/elasticsearchexporter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,16 @@ type mappingModel interface {
encodeSpan(pcommon.Resource, string, ptrace.Span, pcommon.InstrumentationScope, string, elasticsearch.Index, *bytes.Buffer) error
encodeSpanEvent(resource pcommon.Resource, resourceSchemaURL string, span ptrace.Span, spanEvent ptrace.SpanEvent, scope pcommon.InstrumentationScope, scopeSchemaURL string, idx elasticsearch.Index, buf *bytes.Buffer)
hashDataPoint(datapoints.DataPoint) uint32
encodeDocument(objmodel.Document, *bytes.Buffer) error
encodeMetrics(resource pcommon.Resource, resourceSchemaURL string, scope pcommon.InstrumentationScope, scopeSchemaURL string, dataPoints []datapoints.DataPoint, validationErrors *[]error, idx elasticsearch.Index, buf *bytes.Buffer) (map[string]string, error)
encodeProfile(pcommon.Resource, pcommon.InstrumentationScope, pprofile.Profile, func(*bytes.Buffer, string, string) error) error
}

// encodeModel tries to keep the event as close to the original open telemetry semantics as is.
// No fields will be mapped by default.
//
// Field deduplication and dedotting of attributes is supported by the encodeModel.
//
// See: https://github.com/open-telemetry/oteps/blob/master/text/logs/0097-log-data-model.md
type encodeModel struct {
dedot bool
mode MappingMode
mode MappingMode
}

const (
Expand All @@ -116,9 +112,7 @@ func (m *encodeModel) encodeLog(resource pcommon.Resource, resourceSchemaURL str
default:
document = m.encodeLogDefaultMode(resource, record, scope, idx)
}
document.Dedup()

return document.Serialize(buf, m.dedot)
return document.Serialize(buf, m.mode == MappingECS)
}

func (m *encodeModel) encodeLogDefaultMode(resource pcommon.Resource, record plog.LogRecord, scope pcommon.InstrumentationScope, idx elasticsearch.Index) objmodel.Document {
Expand Down Expand Up @@ -195,16 +189,6 @@ func (m *encodeModel) encodeLogECSMode(resource pcommon.Resource, record plog.Lo
return document
}

func (m *encodeModel) encodeDocument(document objmodel.Document, buf *bytes.Buffer) error {
document.Dedup()

err := document.Serialize(buf, m.dedot)
if err != nil {
return err
}
return nil
}

// upsertMetricDataPointValue upserts a datapoint value to documents which is already hashed by resource and index
func (m *encodeModel) hashDataPoint(dp datapoints.DataPoint) uint32 {
switch m.mode {
Expand Down Expand Up @@ -232,7 +216,7 @@ func (m *encodeModel) encodeDataPointsECSMode(resource pcommon.Resource, dataPoi
}
document.AddAttribute(dp.Metric().Name(), value)
}
err := m.encodeDocument(document, buf)
err := document.Serialize(buf, true)

return document.DynamicTemplates(), err
}
Expand Down Expand Up @@ -262,9 +246,7 @@ func (m *encodeModel) encodeSpan(resource pcommon.Resource, resourceSchemaURL st
default:
document = m.encodeSpanDefaultMode(resource, span, scope, idx)
}
document.Dedup()
err := document.Serialize(buf, m.dedot)
return err
return document.Serialize(buf, m.mode == MappingECS)
}

func (m *encodeModel) encodeSpanDefaultMode(resource pcommon.Resource, span ptrace.Span, scope pcommon.InstrumentationScope, idx elasticsearch.Index) objmodel.Document {
Expand Down
Loading

0 comments on commit ee5078a

Please sign in to comment.