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

Allow cloning of ReadWriteLogRecord #4090

Merged
merged 9 commits into from
Jul 26, 2024

Conversation

pellared
Copy link
Member

@pellared pellared commented Jun 28, 2024

Follows #4062

Why

  1. There is a need to make a deep copy of a ReadWriteRecord in order to implement an experimental isolating processor outside of the SDK (as it is experimental, we prefer to land it to contrib repository first).
  2. Allow fine-grained control for the users to make a deep copy only when necessary. This allows users to have more control on the processing e.g. pass a clone to asynchronous processor to avoid race conditions or make performance improvements.

Pre-work

@pellared pellared requested review from a team June 28, 2024 10:53
@pellared pellared self-assigned this Jun 28, 2024
@pellared pellared added the spec:logs Related to the specification/logs directory label Jun 28, 2024
specification/logs/sdk.md Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jul 6, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 6, 2024
@pellared pellared changed the title Allow adding Clone method to ReadWriteLogRecord Allow cloning of ReadWriteLogRecord Jul 9, 2024
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jul 18, 2024
@pellared pellared requested a review from cijothomas July 18, 2024 18:33
@carlosalberto
Copy link
Contributor

So IMHO we should merge this, specially after we got @cijothomas's review. A note on why this is added would help? i.e. regarding helping implementing #4062 if applicable.

@pellared
Copy link
Member Author

pellared commented Jul 25, 2024

A note on why this is added would help?

In the PR description or the specification itself?
It is already there in PR description.

@carlosalberto
Copy link
Contributor

In the Spec ;)

@pellared
Copy link
Member Author

@carlosalberto PTAL c898c54

@pellared pellared removed the Stale label Jul 25, 2024
@carlosalberto carlosalberto merged commit 1c7eb21 into open-telemetry:main Jul 26, 2024
6 checks passed
@pellared pellared deleted the log-clone branch July 26, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:logs Related to the specification/logs directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants