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

Experimental fixes #604

Merged
merged 9 commits into from
Aug 23, 2023
Merged

Conversation

albertored
Copy link
Contributor

@albertored albertored commented Aug 17, 2023

A couple of improvements on the experimental applications.

Different contributions are split in different commits so if you prefer to accept only some of them I can discard the unwanted ones:

  • Improvement on typespecs
  • Easier way for creating observable instrumenters without callback
  • Add instrument_unit to view criteria (see spec)
  • Validate instrument name (see spec)

@albertored albertored requested a review from a team August 17, 2023 16:32
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (23c49ec) 72.26% compared to head (ca16631) 72.26%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #604   +/-   ##
=======================================
  Coverage   72.26%   72.26%           
=======================================
  Files          60       60           
  Lines        1904     1904           
=======================================
  Hits         1376     1376           
  Misses        528      528           
Flag Coverage Δ
api 68.06% <ø> (ø)
elixir 18.31% <ø> (ø)
erlang 73.58% <ø> (ø)
exporter 66.80% <ø> (ø)
sdk 78.13% <ø> (ø)
zipkin 54.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@albertored
Copy link
Contributor Author

@tsloughter review suggestion applied and rebased on main, tests pass locally

@albertored albertored requested a review from tsloughter August 22, 2023 17:22
@albertored
Copy link
Contributor Author

I think the failing test is a little bit flaky, it fails (not always) also on different PR (see for instance #609)

@tsloughter
Copy link
Member

Yup, I'm rerunning it.Should be fine.

@tsloughter tsloughter merged commit 9bbe0b5 into open-telemetry:main Aug 23, 2023
@tsloughter
Copy link
Member

Hey, can you send another PR with an update to the changelog? Sorry, I always forget to check this until I hit the merge button :)

@albertored
Copy link
Contributor Author

Also changes to experimental apps go into the main changelog?

@albertored albertored deleted the experimental-fixes branch August 23, 2023 12:58
@tsloughter
Copy link
Member

Yea, just under their names, so like Experimental API is the heading.

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.

2 participants