feat: Introduce policy to received_bytes
#16056
Conversation
DylanGuedes
force-pushed
the
introduce-policy-to-received-bytes
branch
2 times, most recently
from
5324437 to
9f8915b
Compare
DylanGuedes
force-pushed
the
introduce-policy-to-received-bytes
branch
from
9f8915b to
fa2f99e
Compare
DylanGuedes
force-pushed
the
introduce-policy-to-received-bytes
branch
from
edf28f0 to
9006a87
Compare
…na/loki into introduce-policy-to-received-bytes
| @@ -111,7 +116,7 @@ func (t *PushTarget) run() error { | |||
| func (t *PushTarget) handleLoki(w http.ResponseWriter, r *http.Request) { | |||
| logger := util_log.WithContext(r.Context(), util_log.Logger) | |||
| userID, _ := tenant.TenantID(r.Context()) | |||
| req, err := push.ParseRequest(logger, userID, r, nil, push.EmptyLimits{}, push.ParseLokiRequest, nil, nil, false) | |||
| req, err := push.ParseRequest(logger, userID, r, nil, push.EmptyLimits{}, push.ParseLokiRequest, nil, t.PolicyResolver, false) | |||
There was a problem hiding this comment.
What about passing nil here? Then the parsers can do:
var policy string
if policyResolver != nil {
policy = policyResolver...
}
pkg/validation/limits.go
Outdated
| @@ -1124,6 +1124,10 @@ func (o *Overrides) EnforcedLabels(userID string) []string { | |||
| return o.getOverridesForUser(userID).EnforcedLabels | |||
| } | |||
|
|
|||
| func (o *Overrides) PolicyFor(userID string, lbs labels.Labels) string { | |||
There was a problem hiding this comment.
I think this one is unused?
| @@ -439,6 +491,10 @@ func (f *fakeLimits) OTLPConfig(_ string) OTLPConfig { | |||
| return DefaultOTLPConfig(defaultGlobalOTLPConfig) | |||
| } | |||
|
|
|||
| func (f *fakeLimits) PolicyFor(_ string, lbs labels.Labels) string { | |||
There was a problem hiding this comment.
If unused, we can remove this one as well
There was a problem hiding this comment.
we need a PolicyFor implementation and i'm using the environment label to tell which policy to use
| expectedStructuredMetadataBytes int | ||
| expectedBytes int | ||
| expectedLines int | ||
| expectedStructuredMetadataBytes map[string]int |
There was a problem hiding this comment.
I think we should add a test here with multiple named and unnamed policies
There was a problem hiding this comment.
Also passing nil as the policyResolver, in which case only one item per map should be expected ("")
pkg/loghttp/push/push.go
Outdated
| } | ||
|
|
||
| totalNumLines := int64(0) |
There was a problem hiding this comment.
nit: this is more idiomatic
var totalNumLines int64
pkg/loghttp/push/otlp.go
Outdated
| @@ -37,20 +37,21 @@ const ( | |||
|
|
|||
| func newPushStats() *Stats { | |||
There was a problem hiding this comment.
nit: I know this one was already here, but I think it makes more sense to move this new function to push.go right below the definition of Stats
There was a problem hiding this comment.
lol good find, totally overlooked that stats were defined in the other file. fixed.
Co-authored-by: Salva Corts <[email protected]> Signed-off-by: Dylan Guedes <[email protected]>
What this PR does / why we need it:
Introduce the
policylabel to the received_bytes metric family to be able to distinguish between different policies ingestion.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR