-
Notifications
You must be signed in to change notification settings - Fork 74
feat: ingestion metrics for system monitoring #2913
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: ingestion metrics for system monitoring #2913
Conversation
61b6aa1 to
1707e1e
Compare
0c76910 to
3e3b809
Compare
josevalim
left a comment
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.
Hi @Ziinc! 👋
Comments inline!
| [label] -> {label, [label]} | ||
| end) | ||
| end) | ||
| |> Map.new() |
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.
For completeness, you could use custom Ecto.Types to store those aleady as lists in the database: https://hexdocs.pm/ecto/Ecto.Type.html - although I am not expecting this code to be performance sensitive enough to matter.
| end | ||
|
|
||
| def get_related_user_id(map) do | ||
| case map do |
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.
This is a separate conversation but I wonder if something could be done to remove the number of "sources" here.
Also, it is worth noting that you are caching whole structs, but you may only need the user_id field. For example, Cache.get_endpoint_query seems to be used exclusively here and you don 't care about the other fields, so there is no need to store or read them. You may benefit from adding a get_user_id_by_endpoint_query. This may also apply to other operations, but please double check!
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.
yes, i intend to refactor that off as it isn't as performant, and we should be able to derive the user_id from wherever the telemetry is getting emitted and set it on the metadata, thereby bypassing the whole need to fetch from Cachex
Co-authored-by: José Valim <[email protected]>
Co-authored-by: José Valim <[email protected]>
4cfbaba to
475ecfb
Compare
System monitoring for users:
:metricssource pre-aggregated.:keepcallback multiple times for each each SourceSupfor user-specific logs, these are routed to the source based on whether specific identifying metadata keys are present. If the log is pertaining to a specific source/backend belonging to the user, then it will get ingested into that source.
4 specific user metrics are implemented
Refactors tests to avoid testing implementation.
Closes ANL-1202