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

feat(loki): include structured_metadata size while asserting rate limit #14571

Conversation

vlad-diachenko
Copy link
Contributor

What this PR does / why we need it:
Include structured_metadata size while asserting rate limit and into loki_distributor_discarded_bytes metric.
This change is required because OTEL logs use structured_metadata more aggressively, and the size of it can be even bigger than the size of the log line.

Also, when the write path is blocked(feature implemented in this PR), the amount of ingested data must be equal to discarded data amount with the reason blocked_ingestion. Before this PR, it was not true, because the metric loki_distributor_bytes_received_total included structured_metadata size but loki_distributor_discarded_bytes did not.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

…it and into `loki_distributor_discarded_bytes` metric.

Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
@vlad-diachenko vlad-diachenko requested a review from a team as a code owner October 22, 2024 11:49
@vlad-diachenko vlad-diachenko changed the title feat(loki): include structured_metadata size while asserting rate limit and into loki_distributor_discarded_bytes metric feat(loki): include structured_metadata size while asserting rate limit Oct 22, 2024
Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Oct 22, 2024
Copy link
Contributor

@JStickler JStickler left a comment

Choose a reason for hiding this comment

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

[docs team] The documentation LGTM.

pkg/ingester-rf1/stream.go Outdated Show resolved Hide resolved
pkg/ingester-rf1/stream.go Outdated Show resolved Hide resolved
pkg/ingester/stream.go Outdated Show resolved Hide resolved
vlad-diachenko and others added 2 commits October 22, 2024 19:09
…ed metadata size

Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
Copy link
Contributor

@salvacorts salvacorts left a comment

Choose a reason for hiding this comment

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

LGTM. As soon as we verify this works in dev, happy to merge it.

vlad-diachenko and others added 2 commits October 24, 2024 18:10
…uture metadata size because it's injected on distributors' size(for most of the cases).

Signed-off-by: Vladyslav Diachenko <vlad.diachenko@grafana.com>
@vlad-diachenko
Copy link
Contributor Author

LGTM. As soon as we verify this works in dev, happy to merge it.

I have verified that the fix works as expected:
image

@vlad-diachenko vlad-diachenko merged commit a962edb into main Oct 25, 2024
61 checks passed
@vlad-diachenko vlad-diachenko deleted the vlad.diachenko/include-structured-metadata-in-discarded-bytes branch October 25, 2024 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants