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

otelzap: Basic implementation of Core.Write #5539

Merged
merged 25 commits into from
May 15, 2024
Merged

otelzap: Basic implementation of Core.Write #5539

merged 25 commits into from
May 15, 2024

Conversation

khushijain21
Copy link
Contributor

@khushijain21 khushijain21 commented May 9, 2024 •
edited
Loading

Part of #5191

Pre-work #5279

This PR implements Write method for Core

@khushijain21 khushijain21 requested a review from pellared as a code owner May 9, 2024 04:27
@khushijain21 khushijain21 requested a review from a team May 9, 2024 04:27
@khushijain21 khushijain21 changed the title Implement functionality of Core otelzap: Implement functionality of Core May 9, 2024
@khushijain21 khushijain21 marked this pull request as draft May 9, 2024 04:32
@codecov Codecov
Copy link

codecov bot commented May 9, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.2%. Comparing base (e536eba) to head (2db5b6b).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5539     +/-   ##
=======================================
+ Coverage   63.1%   63.2%   +0.1%     
=======================================
  Files        193     193             
  Lines      11892   11917     +25     
=======================================
+ Hits        7504    7534     +30     
+ Misses      4171    4166      -5     
  Partials     217     217
Files Coverage Δ
bridges/otelzap/core.go 92.7% <100.0%> (+15.4%) ⬆️

@khushijain21 khushijain21 marked this pull request as ready for review May 9, 2024 04:48
pellared
pellared reviewed May 10, 2024
View reviewed changes
Copy link
Member

@pellared pellared 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.

Can you please change this PR so that it just partially implements Write? Can you remove the code related to Enabled, Check, With?

bridges/otelzap/core.go Outdated Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
@khushijain21 khushijain21 changed the title otelzap: Implement functionality of Core otelzap: Implement Write method of Core May 13, 2024
@khushijain21 khushijain21 changed the title otelzap: Implement Write method of Core otelzap: Implement write method for Core May 13, 2024
@khushijain21 khushijain21 changed the title otelzap: Implement write method for Core otelzap: Implement write method for Core May 13, 2024
@khushijain21 khushijain21 requested a review from pellared May 13, 2024 04:45
pellared
pellared reviewed May 15, 2024
View reviewed changes
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Overall LGTM

bridges/otelzap/core.go Outdated Show resolved Hide resolved
bridges/otelzap/core.go Outdated Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
@khushijain21 khushijain21 requested a review from pellared May 15, 2024 13:45
@pellared pellared added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label May 15, 2024
@pellared pellared changed the title otelzap: Implement write method for Core otelzap: Basic implementation of Core.Write May 15, 2024
@pellared pellared mentioned this pull request May 15, 2024
Closed
dmathieu
dmathieu reviewed May 15, 2024
View reviewed changes
bridges/otelzap/core.go Show resolved Hide resolved
bridges/otelzap/core_test.go Show resolved Hide resolved
bridges/otelzap/core_test.go Outdated Show resolved Hide resolved
@pellared pellared requested a review from dmathieu May 15, 2024 15:44
@pellared pellared merged commit 8199709 into open-telemetry:main May 15, 2024
23 checks passed
@pellared
Copy link
Member

Thanks @khushijain21 👍

zailic pushed a commit to zailic/opentelemetry-go-contrib that referenced this pull request May 20, 2024
@khushijain21 @zailic
otelzap: Basic implementation of Core.Write (open-telemetry#5539)
c05740c
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pellared pellared pellared approved these changes

@MrAlias MrAlias MrAlias approved these changes

@dmathieu dmathieu dmathieu approved these changes

Assignees
No one assigned
Labels
Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG
Projects
None yet
Milestone
untracked
Development

Successfully merging this pull request may close these issues.
4 participants
@khushijain21 @pellared @dmathieu @MrAlias