Skip to content

feat: add enforceLimits to ingest_limits.go - #17117

Merged
grobinson-grafana merged 1 commit into
mainfrom
grobinson/add-enforce-limits
Apr 11, 2025
Merged

feat: add enforceLimits to ingest_limits.go#17117
grobinson-grafana merged 1 commit into
mainfrom
grobinson/add-enforce-limits

Conversation

@grobinson-grafana

@grobinson-grafana grobinson-grafana commented Apr 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This pull request adds a new method to ingest_limits.go to enforce limits against a slice of streams. It returns the streams within the limits, and in the case where one or more streams exceed per-tenant limits, the reasons those streams were not included in the result.

This pull request is stacked on top of #17112.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

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
@grobinson-grafana
grobinson-grafana requested a review from a team as a code owner April 10, 2025 22:14
@grobinson-grafana
feat: add enforceLimits to ingest_limits.go
2c9a327 
This commit adds a new method to ingest_limits.go to enforce limits
against a slice of streams. It returns the streams within the limits,
and in the case where one or more streams exceed per-tenant limits,
the reasons those streams were not included in the result.
@grobinson-grafana
grobinson-grafana force-pushed the grobinson/add-enforce-limits branch from 8515f9e to 2c9a327 Compare April 11, 2025 08:23
@grobinson-grafana
grobinson-grafana merged commit d197cda into main Apr 11, 2025
@grobinson-grafana
grobinson-grafana deleted the grobinson/add-enforce-limits branch April 11, 2025 08:33
trevorwhitney added a commit that referenced this pull request Apr 11, 2025
@trevorwhitney
Squashed commit of the following:
eb615eb 
commit 7e9be1148c3154fba157c4d543c49846261b2d73
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Fri Apr 11 11:56:36 2025 -0600

    Squashed commit of the following:

    commit bd43313
    Author: Robert Fratto <robertfratto@gmail.com>
    Date:   Fri Apr 11 09:52:42 2025 -0400

        chore: align benchmark results between chunks and dataobjs (#17127)

    commit 88beefb
    Author: Robert Fratto <robertfratto@gmail.com>
    Date:   Fri Apr 11 08:09:33 2025 -0400

        fix(logql): Fix inconsistency with parsed field short circuiting (#17104)

        PR #8724 originally changed the behaviour of LogQL so that the first extracted
        field takes precedence over any later extracted field with the same name.

        However, this PR had two subtle issues due to the caching that parse stages
        perform for key lookups:

        * The precedence logic did not always apply to duplicate fields within the same
          parse stage (notably for logfmt).

        * The caching behaviour could incorrectly cause stages to permanently ignore
          fields for future log lines where that stage should otherwise perform
          extraction.

        Additionally, once structured metadata was introduced, it was incorrectly being
        flagged as an "extracted field." Combined with the caching behaviour described
        above, this means that parsed fields only take precedence over structured
        metadata if the first log line encountered by the engine doesn't have that
        field also set as structured metadata.

        This can be demonstrated with the following scenario:

        1. A log line from a stream that uses structured metadata for `trace_id` is
           encountered first. This (incorrectly) causes the trace_id field to be
           ignored in all parse stages.

        2. All later log lines from streams without structured metadata can no longer
           extract or filter on a parsed `trace_id` for the lifetime of the query, due
           to the caching from step 1.

        The fix is two-fold:

        * Decouple skipping previously extracted fields from the logic which caches
          interned keys.

        * Only mark parsed labels as extracted fields (previously all labels).

        Because the check for whether a field is already extracted is now called more
        times, the list of extracted fields has been updated to a map to allow for
        faster checks.

    commit 7312ccb
    Author: Karsten Jeschkies <karsten.jeschkies@grafana.com>
    Date:   Fri Apr 11 11:46:23 2025 +0200

        refactor(stringlabels): Support stringlabels distributor tests (#17123)

    commit 5bba0c2
    Author: Karsten Jeschkies <karsten.jeschkies@grafana.com>
    Date:   Fri Apr 11 11:06:04 2025 +0200

        refactor(stringlabels): Support stringlabels ingester tests (#17121)

    commit 8e42118
    Author: benclive <ben.clive@grafana.com>
    Date:   Fri Apr 11 09:47:52 2025 +0100

        chore: Collect basic stats from dataobj queries (#17111)

    commit d197cda
    Author: George Robinson <george.robinson@grafana.com>
    Date:   Fri Apr 11 09:33:36 2025 +0100

        feat: add enforceLimits to ingest_limits.go (#17117)

    commit b0931b1
    Author: George Robinson <george.robinson@grafana.com>
    Date:   Fri Apr 11 09:23:27 2025 +0100

        chore: change the result of exceedsLimits (#17112)

    commit bcceded
    Author: George Robinson <george.robinson@grafana.com>
    Date:   Fri Apr 11 09:09:12 2025 +0100

        chore: fix private interface had exported methods (#17115)

    commit 75593e0
    Author: Karsten Jeschkies <karsten.jeschkies@grafana.com>
    Date:   Fri Apr 11 07:41:52 2025 +0200

        refactor(stringlabels): Support stringlabels in loghttp, pattern and ruler tests. (#17102)

commit e616f89e9dca98f4341c54587f5f71759f2433bb
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu Apr 10 14:58:00 2025 -0600

    fix: lint and format

commit e52bca19f611b5107ac56f93a426429404935e30
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu Apr 10 12:54:32 2025 -0600

    fix: fix a few more signatures

commit 6c5cbde7e290146ca33ffc5300f9343b1f8a8de0
Merge: b289991170 c7ffeb5
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu Apr 10 12:11:49 2025 -0600

    Merge branch 'main' into refactor-extractors-multiple-samples

commit b2899911701762e81348611da3d72197477594ef
Merge: 9bc6909 2aed4c3
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Apr 8 15:29:34 2025 -0600

    Merge branch 'main' into refactor-extractors-multiple-samples

commit 9bc6909
Author: Trevort Whitney <trevorjwhitney@gmail.com>
Date:   Tue Apr 8 09:57:28 2025 -0600

    refactor: extractors can return multiple samples

commit 4c44821
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Apr 7 11:28:28 2025 -0600

    fix: slices.Delete usage

commit b150156
Merge: b1b41c5 9e9f534
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Apr 7 10:46:25 2025 -0600

    Merge branch 'main' into multi-variant-series-limits

commit b1b41c5
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Apr 7 10:45:53 2025 -0600

    refactor: apply feedback from review

commit d668449
Merge: 63911a5 0a3230f
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Apr 1 11:53:26 2025 -0600

    Merge branch 'main' into multi-variant-series-limits

commit 63911a5
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Apr 1 11:48:30 2025 -0600

    perf: mutate response data in place

commit c16f134
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Thu Mar 27 17:23:13 2025 -0600

    feat: return varaints not exceeding limits

commit 127f29e
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Mar 25 17:15:08 2025 -0600

    fix: lint and format

commit b534760
Merge: d45a53a c40053a
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Mar 25 17:02:29 2025 -0600

    Merge branch 'main' into multi-variant-series-limits

commit d45a53a
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Tue Mar 25 17:01:31 2025 -0600

    feat: fix limit enforcement in step evaluator

commit 2202364
Merge: 8ffc96d 0497c6b
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Mar 24 14:23:31 2025 -0600

    Merge branch 'main' into multi-variant-series-limits

commit 8ffc96d
Author: Trevor Whitney <trevorjwhitney@gmail.com>
Date:   Mon Mar 24 14:16:29 2025 -0600

    chore: apply series limit per variant for MV queries
@loki-gh-app loki-gh-app Bot mentioned this pull request Jun 2, 2025
Closed
This was referenced Jun 24, 2025
Closed
Closed
This was referenced Jul 7, 2025
Closed
Closed
@loki-gh-app loki-gh-app Bot mentioned this pull request Jul 21, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L

2 participants

@grobinson-grafana @periklis