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

Add log attribute limit configuration #2861

Merged

Conversation

alanwest
Copy link
Member

@alanwest alanwest commented Oct 10, 2022

Fixes #2860

Adds log attribute limit configuration.

These new environment variables bring more consistency between spans and logs. Therefore, I believe it should be exempt from the moratorium on new env vars.

@github-actions
Copy link

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

@github-actions
Copy link

github-actions bot commented Nov 4, 2022

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 4, 2022
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 29, 2022
@djaglowski djaglowski removed the Stale label Nov 29, 2022
@djaglowski djaglowski reopened this Nov 29, 2022
@arminru arminru added area:sdk Related to the SDK area:configuration Related to configuring the SDK spec:logs Related to the specification/logs directory labels Dec 6, 2022
@jack-berg
Copy link
Member

jack-berg commented Dec 6, 2022

I believe we reached agreement on the naming question of *_LOG_* vs. *_LOGRECORD_*.

@open-telemetry/specs-logs-approvers please take a look.

edit
Also we briefly discussed in the 12/6/22 Spec SIG and @carlosalberto and @arminru mentioned that this is probably exempt from the moratorium because it doesn't introduce any new concepts and brings the log signal in line with the the other signals.

specification/logs/sdk.md Outdated Show resolved Hide resolved
@github-actions github-actions bot closed this Dec 23, 2022
@tigrannajaryan tigrannajaryan reopened this Jan 3, 2023
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@tigrannajaryan
Copy link
Member

This includes changes to trace sdk file. Either fix the the PR description to explain why it is in one PR with the log limits or split into 2 separate PRs if unrelated.

@tigrannajaryan
Copy link
Member

@open-telemetry/specs-approvers please review.

Copy link
Member Author

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

This includes changes to trace sdk file. Either fix the the PR description to explain why it is in one PR with the log limits or split into 2 separate PRs if unrelated.

I'm fine splitting the changes to the trace SDK into a separate PR. Though, the corresponding sections of log and trace SDK should mirror each other. I think it makes sense to make the changes in trace and log specs in parallel to eliminate the possibility of continued divergence.

Below is my thought process for each of the changes I've made to the trace SDK.

specification/trace/sdk.md Outdated Show resolved Hide resolved
specification/trace/sdk.md Outdated Show resolved Hide resolved
specification/trace/sdk.md Outdated Show resolved Hide resolved
@alanwest alanwest changed the title Add attribute limit configuration for log records Add log attribute limit configuration - refactor corresponding part of trace SDK to match Jan 18, 2023
@alanwest alanwest changed the title Add log attribute limit configuration - refactor corresponding part of trace SDK to match Add log attribute limit configuration Jan 20, 2023
@tigrannajaryan tigrannajaryan requested a review from a team January 23, 2023 18:59
@tigrannajaryan
Copy link
Member

Has enough approvals, 2 days passed since last change, merging.

@tigrannajaryan tigrannajaryan merged commit 273a100 into open-telemetry:main Jan 27, 2023
@alanwest alanwest deleted the alanwest/log-attribute-limits branch January 27, 2023 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:configuration Related to configuring the SDK area:sdk Related to the SDK spec:logs Related to the specification/logs directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify that attribute limit configuration also applies to LogRecords
8 participants