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

adding generic attributes interface #217

Merged
merged 5 commits into from
Nov 7, 2023
Merged

Conversation

varkey98
Copy link
Contributor

@varkey98 varkey98 commented Nov 5, 2023

Description

Adding a generic GetAll method for attributes so that all the attributes can be queried at once

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Documentation

Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.

@varkey98
Copy link
Contributor Author

varkey98 commented Nov 5, 2023

We would need to use fmt.Sprintf before passing the values to the filter, shall we just store the attributes as strings itself in that case? @ryanericson @tim-mwangi

Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #217 (d9abff1) into main (bf8e5c3) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
+ Coverage   58.81%   58.94%   +0.12%     
==========================================
  Files          55       55              
  Lines        2229     2236       +7     
==========================================
+ Hits         1311     1318       +7     
  Misses        859      859              
  Partials       59       59              
Files Coverage Δ
instrumentation/opentelemetry/span.go 90.32% <100.00%> (+0.78%) ⬆️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link
Collaborator

@tim-mwangi tim-mwangi left a comment

Choose a reason for hiding this comment

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

lgtm. @ryanericson please take a look as well since you were the original author for this :)

@tim-mwangi tim-mwangi merged commit ea08a0b into hypertrace:main Nov 7, 2023
4 of 6 checks passed
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.

3 participants