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

feat(sdk-node): logs support added #3969

Merged
merged 7 commits into from
Jul 10, 2023

Conversation

psk001
Copy link
Contributor

@psk001 psk001 commented Jul 6, 2023

Which problem is this PR solving?

Logs support is added in sdk-node package.

Fixes #3826

Short description of the changes

Dependencies added in package.json
Feature added in sdk file
Tests added

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Added unit tests

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@psk001 psk001 requested a review from a team July 6, 2023 12:51
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks like something went wrong with the new versions we published a few moments ago.

experimental/packages/opentelemetry-sdk-node/package.json Outdated Show resolved Hide resolved
experimental/packages/opentelemetry-sdk-node/package.json Outdated Show resolved Hide resolved
experimental/packages/opentelemetry-sdk-node/package.json Outdated Show resolved Hide resolved
@psk001
Copy link
Contributor Author

psk001 commented Jul 6, 2023

made all required changes

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #3969 (dd269b0) into main (fc28665) will increase coverage by 0.02%.
The diff coverage is 90.90%.

❗ Current head dd269b0 differs from pull request most recent head 9127284. Consider uploading reports for the commit 9127284 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3969      +/-   ##
==========================================
+ Coverage   92.29%   92.31%   +0.02%     
==========================================
  Files         320      321       +1     
  Lines        9145     9189      +44     
  Branches     1939     1953      +14     
==========================================
+ Hits         8440     8483      +43     
- Misses        705      706       +1     
Impacted Files Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts 92.74% <90.90%> (-0.40%) ⬇️

... and 2 files with indirect coverage changes

@pichlermarc
Copy link
Member

Please also add an entry in experimental/CHANGLOG.md 🙂

@pksunkara
Copy link

Please link #3826 to this.

@pksunkara pksunkara mentioned this pull request Jul 7, 2023
8 tasks
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good, a few more comments to address then we can get this merged. 🙂

experimental/packages/opentelemetry-sdk-node/src/sdk.ts Outdated Show resolved Hide resolved
experimental/packages/opentelemetry-sdk-node/src/sdk.ts Outdated Show resolved Hide resolved
experimental/packages/opentelemetry-sdk-node/src/sdk.ts Outdated Show resolved Hide resolved
experimental/CHANGELOG.md Outdated Show resolved Hide resolved
@psk001
Copy link
Contributor Author

psk001 commented Jul 10, 2023

thanks for suggestions @pichlermarc , made all required changes

experimental/CHANGELOG.md Outdated Show resolved Hide resolved
experimental/CHANGELOG.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for logs in sdk-node
3 participants