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: logs api #203

Merged
merged 9 commits into from
Jan 15, 2025
Merged

feat: logs api #203

merged 9 commits into from
Jan 15, 2025

Conversation

yuzurihaaa
Copy link
Contributor

@yuzurihaaa yuzurihaaa commented Jan 10, 2025

Which problem is this PR solving?

  1. This is a continuation of feat: proto logs #202. Please Review that PR first before continuing on this PR due to huge change on proto generated code from the mentioned PR.

Fixes # (issue)

Short description of the change

  1. Add OpenTelemetry Logs API layer.
  2. I am trying to make it as close as possible to https://www.npmjs.com/package/@opentelemetry/api-logs
  3. I export them in experimental_api.dart as in js, this feature is still in experimental, but experimental_api.dart is placed under lib/src/ instead of lib/ which will require defining import package:opentelemetry/lib/src/experimental_api.dart when need it. Let me know if I should place them under sdk.dart

How Has This Been Tested?

  1. Unit test.

Checklist:

  • Unit tests have been added
  • Documentation has been updated

@yuzurihaaa yuzurihaaa changed the title Feat/yr/api feat: logs api Jan 10, 2025
@yuzurihaaa yuzurihaaa mentioned this pull request Jan 10, 2025
2 tasks
@yuzurihaaa yuzurihaaa force-pushed the feat/yr/api branch 2 times, most recently from aa95d5a to cc598cb Compare January 14, 2025 15:42
@yuzurihaaa
Copy link
Contributor Author

branch rebased from latest master

@yuzurihaaa
Copy link
Contributor Author

yuzurihaaa commented Jan 15, 2025

Hi @blakeroberts-wk , about unit test, seems like the part that is causing it to fail is

warning • lib/src/api/context/context.dart:22:18 • The declaration '_contextStackKey' isn't referenced. Try removing the declaration of '_contextStackKey'. •
          unused_element

I ran make analyze on local and I got the same error make: *** [analyze] Error 2

removing final ContextKey _contextStackKey = ContextKey(); , run make analyze and the error no more. I create another PR to fix CI when running make analyze.

@yuzurihaaa
Copy link
Contributor Author

rebased with latest master

@blakeroberts-wk
Copy link
Contributor

QA +1 experimental public or private API additions only, CI is sufficient

@blakeroberts-wk
Copy link
Contributor

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@btr-rmconsole-3 btr-rmconsole-3 bot merged commit 0a6059b into Workiva:master Jan 15, 2025
3 checks passed
@yuzurihaaa yuzurihaaa deleted the feat/yr/api branch January 15, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants