Kv parser #307

jsirianni · 2021-11-16T21:13:52Z

codecov · 2021-11-16T21:16:05Z

Codecov Report

Merging #307 (2c6ae2f) into main (962bfd8) will increase coverage by 0.2%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #307     +/-   ##
=======================================
+ Coverage   77.1%   77.4%   +0.2%     
=======================================
  Files         94      95      +1     
  Lines       4471    4528     +57     
=======================================
+ Hits        3451    3506     +55     
- Misses       701     702      +1     
- Partials     319     320      +1

Impacted Files	Coverage Δ
operator/builtin/parser/keyvalue/keyvalue.go	`100.0% <100.0%> (ø)`
operator/builtin/input/tcp/tcp.go	`78.0% <0.0%> (-1.7%)`	⬇️

djaglowski · 2021-12-03T14:10:41Z

operator/builtin/parser/keyvalue/keyvalue.go

+	parsed := make(map[string]interface{})
+
+	var err error
+	for _, raw := range splitStringByWhitespace(input) {


Linking to my comment on the other PR

Done. The operator now takes a func(input string) []string for splitting the input string. By default, it will split on whitespace, if the user provides a character or substring, stirngs.Split() is used.

operator/builtin/parser/keyvalue/keyvalue.go

djaglowski · 2022-01-25T18:13:36Z

docs/operators/key_value_parser.md

+| `parse_from`     | $                   | A [field](/docs/types/field.md) that indicates the field to be parsed into key value pairs                                                                                                                                               |
+| `parse_to`       | $                   | A [field](/docs/types/field.md) that indicates the field to be parsed as into key value pairs                                                                                                                                            |


The library technically still interprets $ as $body, but we've gotten away from documenting it this way. Better to be clear.

djaglowski · 2022-01-25T18:14:06Z

docs/operators/key_value_parser.md

+
+### Configuration Fields
+
+| Field            | Default             | Description                                                                                                                                                                                                                              |


Nit: To be consistent with other docs, can you add punctuation to the descriptions.

djaglowski · 2022-01-25T18:17:42Z

operator/builtin/parser/keyvalue/keyvalue.go

+	return &KVParserConfig{
+		ParserConfig:  helper.NewParserConfig(operatorID, "key_value_parser"),
+		Delimiter:     "=",
+		PairDelimiter: "",


Suggested change

PairDelimiter: "",

Since this is the zero value for any string, not needed unless it adds clarity, but I don't think it does.

djaglowski · 2022-01-25T18:22:58Z

operator/builtin/parser/keyvalue/keyvalue.go

+		key := cleanString(m[0])
+		value := cleanString(m[1])
+
+		// TODO: Check if key already exists and fail if so?


Failing entirely seems harsh. Probably a lot better to get a partial log than none.

We could alternately log the error. Not sure the performance hit of checking every time is worth it for the rare case when it matters though.

Up to you, but we should decide and remove the TODO.

djaglowski · 2022-01-25T18:24:16Z

operator/builtin/parser/keyvalue/testdata/default.yaml

@@ -0,0 +1 @@
+type: key_value_parser


Suggested change

type: key_value_parser

type: key_value_parser

djaglowski · 2022-01-25T18:24:21Z

operator/builtin/parser/keyvalue/testdata/delimiter.yaml

@@ -0,0 +1,2 @@
+type: key_value_parser
+delimiter: ";"


Suggested change

delimiter: ";"

delimiter: ";"

We're not consistent on this in this library, but we need to move in that direction.

djaglowski · 2022-01-25T18:25:24Z

Overall this looks good to me. Just a few minor cleanup things.

This test is meant to ensure that partial logs are flushed on an interval if they are not 'completed'. The test appears to have too unforgiving a margin when ensuring that the timeout is not applied too soon.

…pen-telemetry#356) Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.17.0 to 1.18.0. - [Release notes](https://github.com/onsi/gomega/releases) - [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md) - [Commits](onsi/gomega@v1.17.0...v1.18.0) --- updated-dependencies: - dependency-name: github.com/onsi/gomega dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.23.2 to 0.23.3. - [Release notes](https://github.com/kubernetes/client-go/releases) - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.23.2...v0.23.3) --- updated-dependencies: - dependency-name: k8s.io/client-go dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…pen-telemetry#363) Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.18.0 to 1.18.1. - [Release notes](https://github.com/onsi/gomega/releases) - [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md) - [Commits](onsi/gomega@v1.18.0...v1.18.1) --- updated-dependencies: - dependency-name: github.com/onsi/gomega dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…try#361) Bumps [go.opentelemetry.io/collector](https://github.com/open-telemetry/opentelemetry-collector) from 0.42.0 to 0.43.1. - [Release notes](https://github.com/open-telemetry/opentelemetry-collector/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-collector@v0.42.0...v0.43.1) --- updated-dependencies: - dependency-name: go.opentelemetry.io/collector dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…etry#362) Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.43.0 to 1.44.0. - [Release notes](https://github.com/golangci/golangci-lint/releases) - [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md) - [Commits](golangci/golangci-lint@v1.43.0...v1.44.0) --- updated-dependencies: - dependency-name: github.com/golangci/golangci-lint dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

) Bumps [github.com/securego/gosec/v2](https://github.com/securego/gosec) from 2.9.6 to 2.10.0. - [Release notes](https://github.com/securego/gosec/releases) - [Changelog](https://github.com/securego/gosec/blob/master/.goreleaser.yml) - [Commits](securego/gosec@v2.9.6...v2.10.0) --- updated-dependencies: - dependency-name: github.com/securego/gosec/v2 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…hes line (open-telemetry#416) * feat(operator/recombine): do not combine logs before first_entry matches line Signed-off-by: Dominik Rosiek <[email protected]>

…try#422) Bumps [go.opentelemetry.io/collector](https://github.com/open-telemetry/opentelemetry-collector) from 0.45.0 to 0.46.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-collector/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-collector@v0.45.0...v0.46.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/collector dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add support for parsing multiline csv records The csv parser previously was unable to properly handle multiline values. This change adds support. Newlines within csv records are preserved.

Several tools were required by this repo but not actually used.

This was a vestigial feature and can be removed without any impact.

* Bump gonum to resolve iteration issue in Go 1.18 * Update changelog for immediate patch

* Revert version of syslog-go * chlog

) Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.7.0 to 1.7.1. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](stretchr/testify@v1.7.0...v1.7.1) --- updated-dependencies: - dependency-name: github.com/stretchr/testify dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…etry#442) Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.44.2 to 1.45.0. - [Release notes](https://github.com/golangci/golangci-lint/releases) - [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md) - [Commits](golangci/golangci-lint@v1.44.2...v1.45.0) --- updated-dependencies: - dependency-name: github.com/golangci/golangci-lint dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…etry#434) * feat(helpers/multiline): add test for flusher if multiple logs in buffer Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): refactor flusher due to bug Signed-off-by: Dominik Rosiek <[email protected]> * feat(helpers/multiline): add more tests and fixes for multiline Signed-off-by: Dominik Rosiek <[email protected]> * feat(helpers/multiline): add more tests and fixes for multiline Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): add flushed function to forceFlusher Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): wrap splitFuncs with force flusher splitFunc to simplify logic Signed-off-by: Dominik Rosiek <[email protected]> * tests(helpers/multiline): unify force flusher starting data length Signed-off-by: Dominik Rosiek <[email protected]> * feat(helpers/multiline): do not flush empty log Signed-off-by: Dominik Rosiek <[email protected]> * tests(helpers/multiline): do not overengineer tests Signed-off-by: Dominik Rosiek <[email protected]> * feat(helpers/multiline): remove enable force flushing Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): remove NewFlusher function Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): flatter flusherSplitFunc Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): extract force flusher related function from flusherSplitFunc Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): remove splitFuncWrapper Signed-off-by: Dominik Rosiek <[email protected]> * tests(helpers/multiline): do not return empty log for NewNewlineSplitFunc Signed-off-by: Dominik Rosiek <[email protected]> * chore(helpers/multiline): add comment Signed-off-by: Dominik Rosiek <[email protected]> * refactor(helpers/multiline): revert not necessary needed change Signed-off-by: Dominik Rosiek <[email protected]>

Resolves open-telemetry#451

* Remove '$' from field syntax * Enforce rule that body fields must start with 'body'

…ons (open-telemetry#372)

This operator assumes that resource and attributes are flat maps. The add operator has made it redundant anyways.

…#397)

* Enforce maximum SD-NAME length of 32, per RFC5424 Longer SD-NAMEs were previously allowed in fork of go-syslog dependency, but it is not clear that this is necessary.

…ry#430) * Add changelog entry for major set of breaking changes.

…lace existing value in the rare case that it already exists

jsirianni added 4 commits November 16, 2021 15:49

contribute key value parser from observiq

bbc73f8

document delimiter param

a6b45c2

make tidy

076fafd

Merge branch 'main' into kv-parser

b87a269

jsirianni requested review from djaglowski and a team November 16, 2021 21:13

djaglowski reviewed Dec 3, 2021

View reviewed changes

jsirianni added 9 commits January 21, 2022 16:26

rebase

8219978

tidy

eba64cd

Merge branch 'open-telemetry-main' into kv-parser

22c34e2

tidy

e00181d

do not support bytes

8f00220

make pair delimiter a configuration option, default to whitespace

5ee14c7

simplify tests and test new pair delimiter option

580ce6b

remove benchmark, not required

373286c

handle single quoted values

427d6ce

jsirianni requested a review from djaglowski January 22, 2022 04:16

jsirianni added 2 commits January 21, 2022 23:31

fail if delimiter and pair delimiter are identical

0857bf7

finish remaining test coverage

2c6ae2f

djaglowski reviewed Jan 25, 2022

View reviewed changes

djaglowski mentioned this pull request Mar 25, 2022

Migrate Key-Value parser from stanza #455

Closed

djaglowski and others added 7 commits March 28, 2022 14:55

Give recombine timeout test a more margin for error (open-telemetry#355)

0b0f8bf

This test is meant to ensure that partial logs are flushed on an interval if they are not 'completed'. The test appears to have too unforgiving a margin when ensuring that the timeout is not applied too soon.

Change log level for missing file case (open-telemetry#357)

7559b93

dependabot bot and others added 28 commits March 28, 2022 14:56

feat(operator/recombine): do not combine logs before first_entry matc…

8bd9504

…hes line (open-telemetry#416) * feat(operator/recombine): do not combine logs before first_entry matches line Signed-off-by: Dominik Rosiek <[email protected]>

Update dependencies (open-telemetry#426)

8064c5b

Add support for parsing multiline csv records (open-telemetry#425)

b3a8528

* Add support for parsing multiline csv records The csv parser previously was unable to properly handle multiline values. This change adds support. Newlines within csv records are preserved.

Update changelog ahead of v0.27.0 release (open-telemetry#427)

13688f4

Update CHANGELOG.md (open-telemetry#432)

48c8d7d

Remove unused tools (open-telemetry#435)

e92a761

Several tools were required by this repo but not actually used.

Remove notion of a default timestamp (open-telemetry#436)

547e79e

This was a vestigial feature and can be removed without any impact.

Bump gonum to resolve iteration issue in Go 1.18 (open-telemetry#437)

f054b8f

* Bump gonum to resolve iteration issue in Go 1.18 * Update changelog for immediate patch

Revert version of syslog-go (open-telemetry#438)

3a4af2e

* Revert version of syslog-go * chlog

Fix typo in recombine operator docs (open-telemetry#452)

5f623db

Resolves open-telemetry#451

Remove '$' from field syntax (open-telemetry#364)

ac38b6d

* Remove '$' from field syntax * Enforce rule that body fields must start with 'body'

Update attribute names used by file_input, to match semantic conventi…

636ffc5

…ons (open-telemetry#372)

Remove restructure operator (open-telemetry#371)

5774206

Remove metadata operator (open-telemetry#429)

28dd828

This operator assumes that resource and attributes are flat maps. The add operator has made it redundant anyways.

Add ObservedTimestamp to entry (open-telemetry#370)

ecefbe3

Add prefixes to many examples in documentation (open-telemetry#453)

7bf6948

Provide a dedicated mechanism for parsing logger name (open-telemetry…

90e31d9

…#397)

Enforce maximum SD-NAME length of 32, per RFC5424 (open-telemetry#439)

104d1b9

* Enforce maximum SD-NAME length of 32, per RFC5424 Longer SD-NAMEs were previously allowed in fork of go-syslog dependency, but it is not clear that this is necessary.

Update Changelog ahead of major set of breaking changes (open-telemet…

b3803d7

…ry#430) * Add changelog entry for major set of breaking changes.

tidy

97c3b23

newline

7f8d2e7

remove default value for empty string. remove TODO, it is okay to rep…

3af7827

…lace existing value in the rare case that it already exists

doc feadback

585f1a0

jsirianni closed this Mar 28, 2022

jsirianni deleted the kv-parser branch March 28, 2022 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kv parser #307

Kv parser #307

jsirianni commented Nov 16, 2021 •

edited

Loading

codecov bot commented Nov 16, 2021 •

edited

Loading

djaglowski Dec 3, 2021

jsirianni Jan 22, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski Jan 25, 2022

djaglowski commented Jan 25, 2022

		\| `parse_from` \| $ \| A [field](/docs/types/field.md) that indicates the field to be parsed into key value pairs \|
		\| `parse_to` \| $ \| A [field](/docs/types/field.md) that indicates the field to be parsed as into key value pairs \|

Kv parser #307

Kv parser #307

Conversation

jsirianni commented Nov 16, 2021 • edited Loading

codecov bot commented Nov 16, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djaglowski commented Jan 25, 2022

jsirianni commented Nov 16, 2021 •

edited

Loading

codecov bot commented Nov 16, 2021 •

edited

Loading