-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: dot support in attribute name #4121
Conversation
Test the formulas once. |
Have tested for formulas. Also update the code to change the update fields fuctions to use Default instead of materialized. |
Can you add a test for formula query? |
Thanks, @srikanthccv , yeah I tested only filters along with formulas, when testing in a group by it broke. Have fixed and added a test as well. |
LGTM now; I can't of think any other case where it can fail. I will take another look and approve. |
Thanks, appreciate it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check time series chart dot attribute names with a limit applied once. Otherwise LGTM
Yeah, right that got left out , have fixed it |
Merged based on Nitya's request |
Summary
This PR adds the following support
.
in order by and group by_
attributes..
in name it is replaced with$$
. Eg:-request.method
becomesattribute_string_request$$method
default
instead of materialziedRelated Issues / PR's
Fixes #3553
please look at SigNoz/signoz-otel-collector#233 first
clickhouse migration : SigNoz/signoz-db-migrations#8
Frontend changes required for this release #3681
Tested
Blast radius
.
it must be updated within a month else it will start failing.