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

Allow wildcard for view instrument name #624

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

albertored
Copy link
Contributor

Complete the implementation of wildcard views

@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

All modified lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

@albertored
Copy link
Contributor Author

@tsloughter by working on this I notice that the otel_view module contains a lot of dialyzer ignores due to the fact that we are using the instrument record in match specs but its type is strictly defined. Now these ignores are starting to percolate outside that module so maybe it's time to extend the instrument type with match specs as already done for aggreagators. The only other solution (beside keeping the ignores) is to use the raw tuple instead of the record when used in match specs, Wat do you think?

@tsloughter
Copy link
Member

@albertored oh yea, and definitely time to extend the type.

@albertored
Copy link
Contributor Author

@tsloughter perfect, I'll add it to this PR if it's ok

@albertored
Copy link
Contributor Author

@tsloughter done, but there is still a failing test

@albertored
Copy link
Contributor Author

albertored commented Oct 2, 2023

@tsloughter rebased to fix conflicts by this time I hit the w3c flakyness :-D

@tsloughter
Copy link
Member

@albertored looks good. rebase on latest main and I'll merge

@albertored
Copy link
Contributor Author

@tsloughter rebased

@tsloughter tsloughter merged commit 2c45b1b into open-telemetry:main Oct 5, 2023
20 checks passed
@albertored albertored deleted the wildcard-view branch October 6, 2023 07:00
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