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

Merged
merged 26 commits into from
Jul 7, 2025
Merged

Kuma Logs #20597

merged 26 commits into from
Jul 7, 2025

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
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
@nubtron nubtron requested a review from audesikorav July 7, 2025 07:10
@nubtron nubtron added this pull request to the merge queue Jul 7, 2025
Copy link
Contributor

@dkirov-dd dkirov-dd left a comment

Choose a reason for hiding this comment

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

Approved changes since last approval was dismissed (e5e9ce3)

Merged via the queue into master with commit eea576f Jul 7, 2025
41 checks passed
@nubtron nubtron deleted the nubtron/kuma-logs branch July 7, 2025 09:42
github-actions bot pushed a commit that referenced this pull request Jul 7, 2025
* Add assets for Kuma logs.

* Update Kuma logs.

* Set app IDs.

* Remove extra line from test.

* Add logs to manifest.json.

* Add saved views.

* Add test results.

* Reformat tests yaml

* Attempt fixing yaml parsing issues.

* Improve formatting.

* Set "kuma" as the pipeline source.

* Add standard attributes to facets.

* Remove kuma-dp source from saved view.

* Remove columns from saved view.

* Remove the service remapper - it doesn't seem to be needed.

* Update support and parsing rules:

* 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

* Restore the service remapper.

* Update tests.

* Add authority helper rule.

* Update test results

* Remove dash for trace id.

* Go back to negated class for kuma.upstream.host since I'm not certain about the format.

* Use "notOpeningParens" for kuma_dp_tcp_log upstream host.

* Remove service remapper. eea576f
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.

6 participants