-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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: return real data in detected fields endpoint #12421
Conversation
@@ -99,17 +99,3 @@ func ParseDetectedLabelsQuery(r *http.Request) (*logproto.DetectedLabelsRequest, | |||
Query: query(r), | |||
}, nil | |||
} | |||
|
|||
func ParseDetectedFieldsQuery(r *http.Request) (*logproto.DetectedFieldsRequest, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to query.go
in the same package
@@ -617,6 +617,93 @@ func ParseVolumeRangeQuery(r *http.Request) (*VolumeRangeQuery, error) { | |||
}, nil | |||
} | |||
|
|||
func ParseDetectedFieldsQuery(r *http.Request) (*logproto.DetectedFieldsRequest, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved from labels.go
and additional fields populated
@@ -900,18 +906,6 @@ func (q *SingleTenantQuerier) Volume(ctx context.Context, req *logproto.VolumeRe | |||
return seriesvolume.Merge(responses, req.Limit), nil | |||
} | |||
|
|||
func (q *SingleTenantQuerier) DetectedFields(_ context.Context, _ *logproto.DetectedFieldsRequest) (*logproto.DetectedFieldsResponse, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved below DetectedLables
so so it was next to the private functions it calls
f0c88e9
to
0f84f6b
Compare
currently not returning any data in |
return logproto.DetectedFieldDuration | ||
} | ||
|
||
if _, err := humanize.ParseBytes(value); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be a string.Contains like we do for the level label detection. I suspect a small benchmark with some special log line will help. Might also be true for duration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the direction !
Alright, got it working locally pointing at
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
9535a13
to
7079f3b
Compare
parse log lines in the querier to return data about detected fields. this is not sustainable long term, but a short term solution to validate the data we want for the frontend.
What this PR does / why we need it:
This PR calculates real responses for the new experimental detected fields endpoint. It does this in an unsustainable, brute force way by parsing each line (up to a limit) in the querier. Ideally, long term, we'll figure out a way to store this data at ingest, but this is a short term solution to vet the API and data we're surfacing to the UI.
Which issue(s) this PR
fixesreferences:RE: #12339