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

added new config to allow kafka record timestamp to set as published_… #47

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

nyk0322
Copy link
Contributor

@nyk0322 nyk0322 commented Apr 24, 2024

…date event attribute

What does this PR do?

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

@nyk0322 nyk0322 requested a review from a team as a code owner April 24, 2024 21:13
Copy link
Contributor

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

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

Thanks for this @nyk0322 ! I left a few comments but this seems like a good addition to me. I might even go so far as to say it should be the default behavior to add the timestamp, but we can start with opt-in for now to preserve the existing behavior, as you have it.

I tested it out locally and observed the records ending up in Datadog with the published_date added.

@nyk0322 nyk0322 requested a review from a team as a code owner April 26, 2024 00:03
Copy link
Contributor

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

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

Thanks @nyk0322 ! This looks good to me now. We'll just need docs approval before merging.

Copy link
Contributor

@estherk15 estherk15 left a comment

Choose a reason for hiding this comment

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

No docs review needed

@jszwedko jszwedko merged commit 052939a into DataDog:master Apr 26, 2024
6 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.

None yet

3 participants