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

[processor/logdeduplicationprocessor] Add logdedupprocessor #34465

Merged

Conversation

MikeGoldsmith
Copy link
Member

Description:

Starts the donation of the logdedupprocessor from ObserveIQ's Bindplane agent on behalf of @BinaryFissionGames.

Link to tracking Issue:

Testing:

Includes unit tests.

Documentation:

@MikeGoldsmith MikeGoldsmith requested review from a team and crobert-1 August 7, 2024 13:04
MikeGoldsmith and others added 3 commits August 7, 2024 14:58
Co-authored-by: Brandon Johnson <binaryfissiongames@gmail.com>
Copy link
Contributor

@BinaryFissionGames BinaryFissionGames left a comment

Choose a reason for hiding this comment

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

This looks good to me as the initial pass, pinging @djaglowski to take a look.

Thanks for taking this on!

processor/logdeduplicationprocessor/metadata.yaml Outdated Show resolved Hide resolved
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

Generally looks good to me. I recognize this is a preliminary commit but I used the opportunity to give it a closer look. My feedback can be set aside for future PRs if there's a preference to have a cleaner copy of the original source.

processor/logdeduplicationprocessor/processor.go Outdated Show resolved Hide resolved
processor/logdeduplicationprocessor/counter.go Outdated Show resolved Hide resolved
processor/logdeduplicationprocessor/counter.go Outdated Show resolved Hide resolved
processor/logdeduplicationprocessor/counter.go Outdated Show resolved Hide resolved
@MikeGoldsmith
Copy link
Member Author

Generally looks good to me. I recognize this is a preliminary commit but I used the opportunity to give it a closer look. My feedback can be set aside for future PRs if there's a preference to have a cleaner copy of the original source.

I wanted to get a approval from @BinaryFissionGames at this PR is on their behalf. After that I also have some suggestions.

I'm happy to address your feedback either in this PR or in subsequent PRs so we can make more isolated changes.

@MikeGoldsmith MikeGoldsmith changed the title [processor/logdeduprocessor] Add logdedupprocessor [processor/logdeduplicationprocessor] Add logdedupprocessor Aug 7, 2024
@MikeGoldsmith
Copy link
Member Author

MikeGoldsmith commented Aug 7, 2024

@djaglowski I think I've addressed all the immediate feedback and created tracking issues for the others:

Feel free to assign these to me.

@djaglowski djaglowski merged commit b92eb43 into open-telemetry:main Aug 7, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Aug 7, 2024
@MikeGoldsmith MikeGoldsmith deleted the observeiq/logdedupe-processor branch August 8, 2024 09:12
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024
…emetry#34465)

**Description:**

Starts the donation of the
[logdedupprocessor](https://github.com/observIQ/bindplane-agent/tree/release/v1.58.0/processor/logdeduplicationprocessor)
from ObserveIQ's Bindplane agent on behalf of @BinaryFissionGames.

**Link to tracking Issue:**

- Closes open-telemetry#34118 

**Testing:**

Includes unit tests.

**Documentation:**

---------

Co-authored-by: Brandon Johnson <binaryfissiongames@gmail.com>
Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
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.

New component: Log deduplication processor
4 participants