-
Notifications
You must be signed in to change notification settings - Fork 146
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
Support flattened data_stream.* fields #3465
Changes from 9 commits
f1fc97c
79e9f0d
c823617
f9aece8
8f1ab98
9c6e325
3706f6c
92b258e
a1c202c
c4e64c4
a3f54f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# Kind can be one of: | ||
# - breaking-change: a change to previously-documented behavior | ||
# - deprecation: functionality that is being removed in a later release | ||
# - bug-fix: fixes a problem in a previous version | ||
# - enhancement: extends functionality but does not break or fix existing behavior | ||
# - feature: new functionality | ||
# - known-issue: problems that we are aware of in a given version | ||
# - security: impacts on the security of a product or a user’s deployment. | ||
# - upgrade: important information for someone upgrading from a prior version | ||
# - other: does not fit into any of the other categories | ||
kind: feature | ||
|
||
# Change summary; a 80ish characters long description of the change. | ||
summary: Support flattened data_stream.* fields in input configuration | ||
|
||
# Long description; in case the summary is not enough to describe the change | ||
# this field accommodate a description without length limits. | ||
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. | ||
description: >- | ||
An input configuration supports flattened fields, however the | ||
'data_stream' field was not being correctly decoded when | ||
flattened. This commit fixes this issue. | ||
|
||
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. | ||
component: elastic-agent | ||
|
||
# PR URL; optional; the PR number that added the changeset. | ||
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. | ||
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. | ||
# Please provide it if you are adding a fragment for a different PR. | ||
pr: https://github.com/elastic/elastic-agent/pull/3465 | ||
|
||
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). | ||
# If not present is automatically filled by the tooling with the issue linked to the PR number. | ||
issue: https://github.com/elastic/elastic-agent/issues/3191 |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -2361,3 +2361,74 @@ func gatherDurationFieldPaths(s interface{}, pathSoFar string) []string { | |||
|
||||
return gatheredPaths | ||||
} | ||||
|
||||
func TestFlattenedDataStream(t *testing.T) { | ||||
expectedNamespace := "test-namespace" | ||||
expectedType := "test-type" | ||||
expectedDataset := "test-dataset" | ||||
|
||||
policy := map[string]any{ | ||||
"outputs": map[string]any{ | ||||
"default": map[string]any{ | ||||
"type": "elasticsearch", | ||||
"enabled": true, | ||||
}, | ||||
}, | ||||
"inputs": []any{ | ||||
map[string]any{ | ||||
"type": "filestream", | ||||
"id": "filestream-0", | ||||
"enabled": true, | ||||
"data_stream.type": expectedType, | ||||
"data_stream.dataset": expectedDataset, | ||||
"data_stream": map[string]any{ | ||||
"namespace": expectedNamespace, | ||||
}, | ||||
}, | ||||
}, | ||||
} | ||||
runtime, err := LoadRuntimeSpecs(filepath.Join("..", "..", "specs"), PlatformDetail{}, SkipBinaryCheck()) | ||||
if err != nil { | ||||
t.Fatalf("cannot load runtime specs: %s", err) | ||||
} | ||||
|
||||
result, err := runtime.ToComponents(policy, nil, logp.DebugLevel, nil) | ||||
if err != nil { | ||||
t.Fatalf("cannot convert policy to component: %s", err) | ||||
} | ||||
|
||||
if len(result) != 1 { | ||||
t.Fatalf("expecting result to have one element, got %d", len(result)) | ||||
} | ||||
|
||||
if len(result[0].Units) != 2 { | ||||
t.Fatalf("expecting result[0].Units to have two elements, got %d", len(result)) | ||||
} | ||||
|
||||
// We do not make assumptions about ordering. | ||||
// Get the input Unit | ||||
var dataStream *proto.DataStream | ||||
for _, unit := range result[0].Units { | ||||
if unit.Err != nil { | ||||
t.Fatalf("unit.Err: %s", unit.Err) | ||||
} | ||||
if unit.Type == client.UnitTypeInput { | ||||
dataStream = unit.Config.DataStream | ||||
break | ||||
} | ||||
} | ||||
|
||||
if dataStream == nil { | ||||
t.Fatal("DataStream cannot be nil") | ||||
} | ||||
|
||||
if dataStream.Dataset != expectedDataset { | ||||
t.Errorf("expecting DataStream.Dataset: %q, got: %q", expectedDataset, dataStream.Dataset) | ||||
} | ||||
if dataStream.Type != expectedType { | ||||
t.Errorf("expecting DataStream.Type: %q, got: %q", expectedType, dataStream.Type) | ||||
} | ||||
if dataStream.Namespace != expectedNamespace { | ||||
t.Errorf("expecting DataStream.Namespace: %q, got: %q", expectedNamespace, dataStream.Namespace) | ||||
} | ||||
Comment on lines
+2408
to
+2433
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can probably be substituted with an assert.ElementsMatch() if we are able to predictably build DataStream objects (since we are building them from protobuf, that's not always a given) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protobuf types do not play well with testify, as far as I know they don't play well with any comparison package that is not built specifically for protobuf. I rather keep the code like that and not have to worry about protobuf internal fields. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should probably be a reason not to propagate protobuf objects across layers if they make writing tests more tedious than it needs to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get that, but it's not too bad either, elastic-agent/pkg/component/component_test.go Line 2110 in 910d17b
|
||||
} |
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.
We can be more compact here