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 call_id, from and to labels to loki logs #562

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

htmluz
Copy link

@htmluz htmluz commented Jan 19, 2025

Building a backend that had a ton of logs with Homer and Loki required adding those labels for performance purposes. Better query times by those labels. Added to rtcp to so it could correlate all call packets with one search by cid.

@CLAassistant
Copy link

CLAassistant commented Jan 19, 2025

CLA assistant check
All committers have signed the CLA.

@lmangani
Copy link
Member

Thanks for taking the time and submitting a PR - we appreciate it! Could you please document the PR with some examples to confirm if the code matches the presented change? And while this might solve an issue it also creates a much bigger one as unfortunately the new labels would introduce high cardinality and its definitely not suitable for all users/setups.

At the very least we need to add a parameter to control the behaviour (like we already do for the IP tags)

@htmluz
Copy link
Author

htmluz commented Jan 19, 2025

Ok I actually hadn't thought about that and am not sure if it will be useful to have the option. After your comment I figured out it's more of a personal need to me to have it since I use a completely different backend to view the calls and then fetch the data from loki, btw I've added the option defaulting to false, in case it fits. thx for your time

@lmangani
Copy link
Member

@htmluz we also use a different backend (qryn/gigapipe) but its always best to be aware of high-cardinality fields particularly for those who might not use this feature. I think it would be even nicer to generalize this and allow any of our extracted fields to be paired with a label in HOMER11

@lmangani lmangani requested a review from adubovikov January 22, 2025 00:10
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