-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Kuma Logs #20597
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
base: master
Are you sure you want to change the base?
Kuma Logs #20597
Conversation
Review from michaelcretzman is dismissed. Related teams and files:
- documentation
- kuma/assets/saved_views/logs_overview.json
Review from michaelcretzman is dismissed. Related teams and files:
- documentation
- kuma/manifest.json
Review from steveny91 is dismissed. Related teams and files:
- agent-integrations
- kuma/manifest.json
kuma/assets/logs/kuma.yaml
Outdated
maybeInt %{regex("(?:-|\\d+)" )} | ||
notBackslash %{regex("[^\\\\]*")} | ||
notColon %{regex("[^:]*")} | ||
notClosingParens %{regex("[^\\)]*")} |
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.
if you expect your services to be always named / formatted this way (e.g edge-gateway_kuma-demo_svc
, redis_kuma-demo_svc_6379
, etc), I'll suggested to scope characters that can be matched with a rule like this
serviceName %{regex("[a-zA-Z0-9_-]*")}
.
Regex will run faster and it will prevent unnecessary backtracking on unexpected content vs using negated class
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.
@audesikorav thanks for the tip! Avoided using negated classes when possible. I kept the negated class for URLs since the alternative seemed pretty complex, and I wasn't sure about the performance impact. Let me know what you think!
kuma/assets/logs/kuma.yaml
Outdated
notQuote %{regex("[^\"]*")} | ||
maybeInt %{regex("(?:-|\\d+)" )} | ||
notBackslash %{regex("[^\\\\]*")} | ||
notColon %{regex("[^:]*")} |
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.
You can remove this one (not used)
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.
Right, removed!
kuma/assets/logs/kuma.yaml
Outdated
# Sample: | ||
# [2025-06-26T21:29:21.535Z] - default 10.42.1.5(unknown)->10.42.1.5:6379(redis_kuma-demo_svc_6379) took 14004ms, sent 6195 bytes, received: 194 bytes | ||
|
||
kuma_dp_tcp_log \[%{isoTimestamp:date}\] %{notSpace:response.flags} %{notSpace:kuma.mesh} %{ipOrHost:kuma.source_address_without_port}\(%{notClosingParens:kuma.source_service}\)\->%{notBackslash:kuma.upstream.host}\(%{notClosingParens:kuma.destination_service}\) took %{integer:duration:scale(1000000)}ms, sent %{integer:network.bytes_written} bytes, received: %{integer:network.bytes_read} bytes |
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.
same for here, if you can replace the negated character and matching only expected characters
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.
Replaced the negated characters, except where I wasn't sure about the format (kuma.mesh) or the regex would have been relatively complex (url).
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.
yep not worry, if you do the format its always best matching less char but its fine if not sure 👍
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.
@audesikorav I saw an instance where the docs appear to contradict their own examples for the format of kuma.upstream.host, I'm not so sure about the format anymore, I'll go back to the negated class for that one to be safe.
kuma/assets/logs/kuma_tests.yaml
Outdated
mesh: "default" | ||
source_address_without_port: "10.42.3.27" | ||
source_service: "unknown" | ||
trace_id: "-" |
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.
are you sure this field is populated as you expected ?
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.
According to the Kuma source code these are either Datadog or zipkin trace IDs. Their macro is '"%REQ(X-B3-TRACEID?X-DATADOG-TRACEID)%"'. Datadog trace IDs are ints if I understand correctly, and zipkin's traces are hex.
I switched to "(unknown|)" or "(-|)" to leave out the placeholder patterns used when no value is available.
kuma/assets/logs/kuma_tests.yaml
Outdated
flags: "NR" | ||
x_envoy_upstream_service_time: "-" | ||
upstream: | ||
host: "-" |
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.
same question here
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.
The docs and my testing indicate <host>:<port>
. However an example in the docs showed tcp://<host>:<port>
. For safety I went back to "notQuote" since I'm not sure the docs are 100% up-to-date.
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.
Oh, and I added "(-|)" to discard placeholders!
* Avoid using negated classes in regexes when possible (except for urls, for which it doesn't seem to be standard practice) * Use rules with alternatives such as "(-|<ITEM>)" or "(unknown|<ITEM>)" to discard placeholder values for empty fields. * Delete unused support rules
… about the format.
Review from audesikorav is dismissed. Related teams and files:
- logs-backend
- kuma/assets/logs/kuma.yaml
- kuma/assets/logs/kuma_tests.yaml
- logs-integrations-reviewers
- kuma/assets/logs/kuma.yaml
- kuma/assets/logs/kuma_tests.yaml
@audesikorav need a moment to update the rules, will tell you when ready again! |
@audesikorav rules updated! |
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.
One nit comment but otherwise seems good on our side !
kuma/assets/logs/kuma.yaml
Outdated
enabled: true | ||
sources: | ||
- date | ||
- type: service-remapper |
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.
are you sure you want to use the source ("kuma") as the official service name ?
(not a blocker if you do, just since you are extracting some service name / maybe the service is already adding service as attribute)
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.
@audesikorav I'm not entirely sure about this one! The reasoning is to have a fallback in case the Agent doesn't set it. But from what I've seen the Agent already sets it to the kubernetes service which is ideal.
I picked the source because I didn't see way to get a service in all variations of the logs. Let me know what you think!
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.
mmh if its already defined by the agent and you want to keep this value then you should remove it, otherwise it will get override by the remapper
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.
@audesikorav Removed it! Thanks for the input!
Review from audesikorav is dismissed. Related teams and files:
- logs-backend
- kuma/assets/logs/kuma.yaml
- logs-integrations-reviewers
- kuma/assets/logs/kuma.yaml
What does this PR do?
This PR adds logs pipelines and saved views for the Kuma integration.
Motivation
This is a step in the creation of the Kuma integration.
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.backport/<branch-name>
label to the PR and it will automatically open a backport PR once this one is merged