Skip to content

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

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

Kuma Logs #20597

wants to merge 25 commits into from

Conversation

nubtron
Copy link
Contributor

@nubtron nubtron commented Jun 26, 2025

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)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

@nubtron nubtron added qa/skip-qa Automatically skip this PR for the next QA changelog/no-changelog labels Jun 27, 2025
@temporal-github-worker-1 temporal-github-worker-1 bot dismissed michaelcretzman’s stale review June 27, 2025 16:59

Review from michaelcretzman is dismissed. Related teams and files:

  • documentation
    • kuma/assets/saved_views/logs_overview.json
steveny91
steveny91 previously approved these changes Jun 27, 2025
@temporal-github-worker-1 temporal-github-worker-1 bot dismissed stale reviews from michaelcretzman and steveny91 July 1, 2025 13:08

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
@nubtron nubtron added assets/no-deploy Prevents APW from deploying this PR in staging and removed assets/no-deploy Prevents APW from deploying this PR in staging labels Jul 2, 2025
maybeInt %{regex("(?:-|\\d+)" )}
notBackslash %{regex("[^\\\\]*")}
notColon %{regex("[^:]*")}
notClosingParens %{regex("[^\\)]*")}

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

Copy link
Contributor Author

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!

notQuote %{regex("[^\"]*")}
maybeInt %{regex("(?:-|\\d+)" )}
notBackslash %{regex("[^\\\\]*")}
notColon %{regex("[^:]*")}

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, removed!

# 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

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

Copy link
Contributor Author

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).

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 👍

Copy link
Contributor Author

@nubtron nubtron Jul 4, 2025

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.

mesh: "default"
source_address_without_port: "10.42.3.27"
source_service: "unknown"
trace_id: "-"

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 ?

Copy link
Contributor Author

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.

flags: "NR"
x_envoy_upstream_service_time: "-"
upstream:
host: "-"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here

Copy link
Contributor Author

@nubtron nubtron Jul 4, 2025

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.

Copy link
Contributor Author

@nubtron nubtron Jul 4, 2025

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!

nubtron added 6 commits July 4, 2025 09:26
* 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
audesikorav
audesikorav previously approved these changes Jul 4, 2025
@audesikorav audesikorav added the assets/deploy-logs-staging ONLY USED BY Logs Backend - Validates that a PR is OK to go to staging label Jul 4, 2025
@temporal-github-worker-1 temporal-github-worker-1 bot dismissed audesikorav’s stale review July 4, 2025 08:59

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
@nubtron
Copy link
Contributor Author

nubtron commented Jul 4, 2025

@audesikorav need a moment to update the rules, will tell you when ready again!

@nubtron
Copy link
Contributor Author

nubtron commented Jul 4, 2025

@audesikorav rules updated!

audesikorav
audesikorav previously approved these changes Jul 4, 2025
Copy link

@audesikorav audesikorav left a 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 !

enabled: true
sources:
- date
- type: service-remapper

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)

Copy link
Contributor Author

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!

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

Copy link
Contributor Author

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!

@temporal-github-worker-1 temporal-github-worker-1 bot dismissed audesikorav’s stale review July 4, 2025 13:48

Review from audesikorav is dismissed. Related teams and files:

  • logs-backend
    • kuma/assets/logs/kuma.yaml
  • logs-integrations-reviewers
    • kuma/assets/logs/kuma.yaml
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.

4 participants