Skip to content
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 otel_logs codec in S3 source (#5028) #5030

Merged
merged 6 commits into from
Oct 11, 2024

Conversation

danhli
Copy link
Contributor

@danhli danhli commented Oct 8, 2024

Description

Add a new codec option, otel_logs, in S3 source. With this option, OTLP JSON files can be ingested and processed natively, similar to ingesting through http endpoint using otel_logs_source.

Issues Resolved

Resolves #5028

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@danhli danhli changed the title Support otlp_json_logs codec in S3 source (#5028) Support opentelemetry_logs codec in S3 source (#5028) Oct 8, 2024
import java.util.function.Consumer;

@DataPrepperPlugin(name = "opentelemetry_logs", pluginType = InputCodec.class)
public class OTLPJsonLogsCodec extends OTLPJsonLogsDecoder implements InputCodec {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to OTelLogsCodec? What was the reason for Json in the name?

Copy link
Contributor Author

@danhli danhli Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed it to OTelLogsInputCodec.

import java.io.InputStream;
import java.util.function.Consumer;

@DataPrepperPlugin(name = "opentelemetry_logs", pluginType = InputCodec.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be otel_json_logs or otel_logs_json?

Copy link
Contributor Author

@danhli danhli Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it to otel_logs.

@danhli danhli changed the title Support opentelemetry_logs codec in S3 source (#5028) Support otel_logs codec in S3 source (#5028) Oct 9, 2024
kkondaka
kkondaka previously approved these changes Oct 9, 2024
Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @danhli . I have a few comments to help with documentation.

* Configuration class for {@link OTelLogsInputCodec}.
*/
public class OTelLogsInputCodecConfig {
static final String JSON_FORMAT = "json";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an enum. It will help with our auto-generated schema and documentation.

I've submitted a few PRs to correct these, such as #5025 and #5035.

Please see these:

https://github.com/opensearch-project/data-prepper/blob/main/data-prepper-plugins/key-value-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/WhitespaceOption.java

https://github.com/opensearch-project/data-prepper/blob/main/data-prepper-plugins/key-value-processor/src/test/java/org/opensearch/dataprepper/plugins/processor/keyvalue/WhitespaceOptionTest.java

This is where it is used:

@JsonProperty("whitespace")
@JsonPropertyDescription("Specifies whether to be lenient or strict with the acceptance of " +
"unnecessary white space surrounding the configured value-split sequence. " +
"In this case, strict means that whitespace is trimmed and lenient means it is retained in the key name and in the value." +
"Default is <code>lenient</code>.")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added OTelLogsFormatOption.

/**
* Configuration class for {@link OTelLogsInputCodec}.
*/
public class OTelLogsInputCodecConfig {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add documentation:

See an example:

@JsonPropertyOrder
@JsonClassDescription("The <code>flatten</code> processor transforms nested objects inside of events into flattened structures.")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

static final String JSON_FORMAT = "json";

@JsonProperty("format")
private String format = JSON_FORMAT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add documentation:

Examples:

@JsonProperty("whitespace")
@JsonPropertyDescription("Specifies whether to be lenient or strict with the acceptance of " +
"unnecessary white space surrounding the configured value-split sequence. " +
"In this case, strict means that whitespace is trimmed and lenient means it is retained in the key name and in the value." +
"Default is <code>lenient</code>.")
@NotNull
private WhitespaceOption whitespace = DEFAULT_WHITESPACE;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@danhli
Copy link
Contributor Author

danhli commented Oct 11, 2024

@dlvenable I have completed the documentation and enum changes. Please review.

Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @danhli for this great contribution!

@dlvenable dlvenable merged commit 8dc66d2 into opensearch-project:main Oct 11, 2024
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support OpenTelemetry logs in S3 source
3 participants