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.

Sign up for GitHub

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

Jump to bottom

Add log attribute limit configuration #2861

Merged
merged 15 commits into from
Jan 27, 2023
Merged

Add log attribute limit configuration #2861

merged 15 commits into from
Jan 27, 2023

Conversation

alanwest
Copy link
Member

@alanwest alanwest commented Oct 10, 2022 •
edited
Loading

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.

@alanwest alanwest requested review from a team October 10, 2022 20:33
@alanwest alanwest mentioned this pull request Oct 10, 2022
Merged
3 tasks
@github-actions
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 Oct 27, 2022
@alanwest alanwest mentioned this pull request Nov 2, 2022
Closed
25 tasks
@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
@djaglowski djaglowski mentioned this pull request Dec 6, 2022
Closed
@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 •
edited
Loading

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.

arminru
arminru approved these changes Dec 6, 2022
View reviewed changes
specification/logs/sdk.md Outdated Show resolved Hide resolved
@tigrannajaryan tigrannajaryan reopened this Jan 3, 2023
@jack-berg jack-berg mentioned this pull request Jan 4, 2023
Merged
@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 Jan 12, 2023
@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.

alanwest
alanwest commented Jan 18, 2023
View reviewed changes
Copy link
Member Author

@alanwest alanwest left a comment •
edited
Loading

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
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
@alanwest
Add log attribute limit configuration (open-telemetry#2861)
970a403 
Fixes open-telemetry#2860

Adds log attribute limit configuration.

These new environment variables bring more consistency between spans and logs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MSNev MSNev MSNev left review comments

@djaglowski djaglowski djaglowski left review comments

@jack-berg jack-berg jack-berg approved these changes

@arminru arminru arminru approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@carlosalberto carlosalberto carlosalberto approved these changes

Assignees

@SergeyKanzhelev SergeyKanzhelev

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify that attribute limit configuration also applies to LogRecords
8 participants
@alanwest @jack-berg @tigrannajaryan @carlosalberto @djaglowski @arminru @MSNev @SergeyKanzhelev