[receiver/kafka] Add JSON-encoded log support #22982
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
djaglowski
changed the title
djaglowski
reviewed
Jun 5, 2023
There was a problem hiding this comment.
Looks reasonable to me but codeowners @pavolloffay and @MovieStoreGuy may have more context.
|
@MovieStoreGuy @pavolloffay Re-pinging for review, need feedback on this |
Co-authored-by: Daniel Jaglowski <[email protected]>
|
@MovieStoreGuy @pavolloffay Re-pinging for review, need feedback on this kafakreceiver enhancement |
djaglowski
approved these changes
Jun 20, 2023
There was a problem hiding this comment.
LGTM.
@Caleb-Hurshman, the lint failure appears unrelated but I can't get the job to cancel. I think rebasing may help.
I would appreciate a review from @MovieStoreGuy or @pavolloffay but I think this is straightforward enough to merge if they do not have an opinion.
djaglowski
force-pushed
the
feat/kafka-support-json-log-encoding
branch
from
f9b2523
to
df6a460
Compare
| t.Parallel() | ||
| um := newJSONLogsUnmarshaler() | ||
| assert.Equal(t, "json", um.Encoding()) | ||
| } |
There was a problem hiding this comment.
Do you mind adding some tests to validate that a given json blob of {blah ...} is encoding as a plog value?
|
|
||
| // get json logs from the buffer | ||
| jsonVal := map[string]interface{}{} | ||
| if err := json.Unmarshal(buf, &jsonVal); err != nil { |
There was a problem hiding this comment.
Could I ask you to use jsoniter instead?
It has improved performance over the standard lib json encoding.
ADDLICENCESE -> ADDLICENSE Signed-off-by: Alex Boten <[email protected]>
found a bunch of double commented out statements Signed-off-by: Alex Boten <[email protected]>
The previous target still exists, but a more intuitive target is added and used by CI. Also switches integration targets to the same iteration pattern used for other groups of modules.
…pen-telemetry#23632) Utilize mdatagen to modify certain metrics from gauge to non-monotonic sum, employing delta aggregation. Adjust the test files to align with the sum test type and update the expected.yaml file for integration testing purposes. Co-authored-by: zeno-splunk <[email protected]>
Add JMX metrics gatherer version `1.27.0-alpha`. cc @open-telemetry/java-contrib-approvers
…nt (open-telemetry#23640) **Description:** updated dataset-go library, send more details in user-agent For more details see https://github.com/scalyr/dataset-go/releases/tag/v0.0.9 **Link to tracking Issue:** open-telemetry#20660 / DSET-3468 **Testing:** verified via unit tests and also e2e with DataSet environment **Documentation:** see https://github.com/scalyr/dataset-go/blob/fbaead7856a9c5669262564dfa4a13aa14a0775d/examples/readme/main.go#L81
…3564) Following the same approach of open-telemetry#20903 I am adding the scope whenever the metric is copied. We could also add the _scope_ at creation time, however, it would change the signature of several functions having already too many parameters. **Link to tracking Issue:** <Issue number if applicable> Fix open-telemetry#23563 **Testing:** Added a test to cover the new use case
…de-references (open-telemetry#23644) Fixes possible panics from unchecked de-references in metrics helper in `awsecscontainermetrics` receiver. **Link to tracking Issue:** aws-observability/aws-otel-collector#2110 **Testing:** Added unit tests for these de-references **Documentation:** n//a
**Description:** <Describe what has changed.> fixed an issue where postgresql receiver was emitting the same attribute value "toast_hit" twice under postgresql.blocks_read metric. **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.>
… hosts that use cgroups v2 (open-telemetry#22706) **Description:** This PR updates the documentation of all the metrics that are available only on cgroups v1 or cgroups v2. It PR also adds two metrics that are only available when the host is using cgroups v2: * `container.memory.anon` * `container.memory.file` Both are documented here: https://docs.kernel.org/admin-guide/cgroup-v2.html#memory-interface-files I enabled the metric `container.memory.file` by default (causing a breaking change) because it is a metric that was already scraped for cgroups v1, contained in the metric `container.memory.total_cache`. ```yaml container.memory.total_cache: enabled: true description: "Total amount of memory used by the processes of this cgroup (and descendants) that can be associated with a block on a block device. Also accounts for memory used by tmpfs (Only available with cgroups v1)." container.memory.file: enabled: true description: "Amount of memory used to cache filesystem data, including tmpfs and shared memory (Only available with cgroups v2)." ``` Let me know your thoughts about this breaking change. **Link to tracking Issue:** open-telemetry#21097 **Testing:** The refactor done during PR open-telemetry#21110 had mocks ready to test these new metrics. There is no need to add more tests. **Documentation:** To see which metrics are available in Docker: * v1: https://github.com/moby/moby/blob/7103efac9d737e8b202126e8c8e2227805e70771/daemon/stats_unix.go#L86 * v2: https://github.com/moby/moby/blob/7103efac9d737e8b202126e8c8e2227805e70771/daemon/stats_unix.go#L195 Kernel cgroup documentation: * v1: https://docs.kernel.org/admin-guide/cgroup-v1/memory.html * v2: https://docs.kernel.org/admin-guide/cgroup-v2.html#memory-interface-files
…-telemetry#23671) Temporarily disable the integration test to unblock the CI. Updates open-telemetry#23670
…or in resource processor (open-telemetry#23253) **Description:** Adding resource_attributes option to every detector in resource detection processor <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> **Link to tracking Issue:** [21482](open-telemetry#21482) **Testing:** Unit test for `resource_attribute` config, adjusting current unit tests to the new implementation **Documentation:** Updated README.md --------- Co-authored-by: Dmitrii Anoshin <[email protected]>
…orrect fingerprint update (open-telemetry#23183) The logic for determining when to update a fingerprint previously failed to account for the case where the buffer size is smaller than the fingerprint. This allows the fingerprint to be truncated in some cases. This PR rewrites the logic to explicitly handle each expected case, with optimization for handling the most common cases first.
…#23675) Fixes open-telemetry#23670 The test appears to have begun failing when the [`latest`](https://hub.docker.com/layers/apache/spark/latest/images/sha256-a1dd2487a97fb5e35c5a5b409e830b501a92919029c62f9a559b13c4f5c50f63?context=explore) tag of for the image was updated. This PR just pins the version.
**Description:** Updates GCP libraries to v0.39.2 and 1.15.2 **Documentation:** updated readmes for new experimental WAL fields
…y#23571) Metric unit is required, see: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/cmd/mdatagen/metadata-schema.yaml#L72-L73. This PR adds the unit to metrics where this is missing: - postgresql.bgwriter.maxwritten - postgresql.table.count
…ts (open-telemetry#23636) This PR adds a `parent` field to the `metadata.yaml`. This is used in subcomponents, and the field contains the type of their parent component. --------- Co-authored-by: Dmitrii Anoshin <[email protected]>
…ry#21185) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Add `container.image.id` and `container.command_line` as optional resource attributes, disabled by default. **Link to tracking Issue:** <Issue number if applicable> Tracking issue open-telemetry#21092 **Testing:** <Describe what testing was performed and which tests were added.> Until this point, all resource attributes have been enabled by default, resulting in the coupling of the [test](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/7bf5d66fae6c1c5360ec2875948f2c2adab6c44f/receiver/dockerstatsreceiver/receiver_test.go#L191) to the default configuration. In order to test the current implementation with the new resource attributes enabled & disabled, a minor refactoring was required in the `TestScrapeV2` test, allowing to pass different configurations for each test. Refer to the following commit 508dbee9aa9a22c1ca79f16bba5abeecceeb9c48 to check the refactor in the test. **Documentation:** <Describe the documentation added.> New resource attributes documentation was generated by `mdatagen`.
codeboten
reviewed
Jul 6, 2023
atoulme
reviewed
Jul 6, 2023
Co-authored-by: Antoine Toulme <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Added support for json encoded logs to the
kafkareceiverLink to tracking Issue: #20734
Testing: Added limited tests for
json_unmarshaler.go, similar to existing 'unmarshaler' files.Documentation: Updated
README.mdof thekafkareceiverto include the JSON encoding option.