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

Make LogRecord immutable, simplifies concurrency #5779

Closed
bogdandrutu opened this issue Sep 4, 2024 · 3 comments
Closed

Make LogRecord immutable, simplifies concurrency #5779

bogdandrutu opened this issue Sep 4, 2024 · 3 comments
Labels
area:logs Part of OpenTelemetry logs enhancement New feature or request pkg:SDK Related to an SDK package

Comments

@bogdandrutu
Copy link
Member

bogdandrutu commented Sep 4, 2024

Problem Statement

I would propose to separate LogRecord struct into mutable vs immutable and pass immutable to Emit to avoid having to deal with concurrency issues.

Proposed Solution

We can achieve this without any heap allocations (interfaces) by having something like:

type LogRecord struct {
  timestamp time.Time
}

func (lr LogRecord) getTimestamp() time.Time {
  return lr.timestamp
}

func (lr LogRecord) toMutable() MutableLogRecord {
  // Here we can to a clone of the non immutable fields.
}

type MutableLogRecord struct {
  LogRecord
}

func (mlr MutableLogRecord) setTimestamp(t time.Time) {
  mlr.timestamp = t
}

func (mlr MutableLogRecord) asImmutable(t time.Time) LogRecord {
  return mlr.LogRecord
}
@bogdandrutu bogdandrutu added the enhancement New feature or request label Sep 4, 2024
@MrAlias
Copy link
Contributor

MrAlias commented Sep 4, 2024

The specification has explicitly stated that the records passed to Emit need to be mutable so the following processors will see all changes made to them:

We originally designed the SDK the way you propose, however, it was not compliant with the specification and there was no desire at the specification level to support the change to make it allowed.

@pellared pellared added pkg:SDK Related to an SDK package area:logs Part of OpenTelemetry logs labels Sep 5, 2024
@Ali-Alnosairi
Copy link

@pellared ,
Does this issue needs to make changes to the specification to support immutable?

@pellared
Copy link
Member

@Ali-Alnosairi, as noted before

We originally designed the SDK the way you propose, however, it was not compliant with the specification and there was no desire at the specification level to support the change to make it allowed.

Yes. However, the proposal was already rejected after a few months of discussions. You can read the Specification SIG meeting notes, watch the recording, check the issues and PRs.

@pellared pellared closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logs Part of OpenTelemetry logs enhancement New feature or request pkg:SDK Related to an SDK package
Projects
None yet
Development

No branches or pull requests

4 participants