Skip to content

[sdk-logs] Add LoggerProviderServiceCollectionBuilder to SDK #4441

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
Merged
CodeBlanch merged 3 commits into from
Apr 26, 2023
Merged

[sdk-logs] Add LoggerProviderServiceCollectionBuilder to SDK #4441

CodeBlanch merged 3 commits into from
Apr 26, 2023

Conversation

@CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Apr 26, 2023

Relates to #4433

Changes

  • Adds LoggerProviderServiceCollectionBuilder to SDK
  • Moves ILogger artifacts into a dedicated folder

Merge requirement checklist

  • CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)

@CodeBlanch CodeBlanch requested a review from a team April 26, 2023 17:00
@CodeBlanch CodeBlanch mentioned this pull request Apr 26, 2023
Open
53 tasks
@utpilla
Copy link
Contributor

utpilla commented Apr 26, 2023

@Yun-Ting The changes in this PR introduced an additional AOT warning. Would you know why would that be the case?

The simple fix to unblock is just to increment the expect count which was made in this commit: 5c68a85.

@CodeBlanch CodeBlanch mentioned this pull request Apr 26, 2023
Closed
@CodeBlanch
Copy link
Member Author

CodeBlanch commented Apr 26, 2023

@utpilla @Yun-Ting I looked into the new warning this PR introduced a bit. Could be tricky to make work with AoT! See: #4445

Decided just to bump the warning count in the test to unblock myself 🤣

@CodeBlanch CodeBlanch merged commit e195dd8 into open-telemetry:main Apr 26, 2023
@CodeBlanch CodeBlanch deleted the sdk-logs-builder branch April 26, 2023 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alanwest alanwest alanwest approved these changes

+1 more reviewer

@vishweshbankwar vishweshbankwar vishweshbankwar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@CodeBlanch @utpilla @alanwest @vishweshbankwar