-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 the jsonpath of json detected fields #16861
feat: return the jsonpath of json detected fields #16861
Conversation
15ce780
to
9f86d57
Compare
func (j *JSONParser) buildJSONPathFromPrefixBuffer() []string { | ||
jsonPath := make([]string, 0, len(j.prefixBuffer)) | ||
for _, part := range j.prefixBuffer { | ||
partStr := unsafe.String(unsafe.SliceData(part), len(part)) // #nosec G103 -- we know the string is not mutated |
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.
the parts of the path will not change, so I've optimized this to use unsafe.String
, which saves ~20% in terms of allocations.
pkg/logql/log/parser.go
Outdated
parserHints ParserHint | ||
keys internedStringSet | ||
parserHints ParserHint | ||
santizedPrefixBuffer []byte |
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.
?
santizedPrefixBuffer []byte | |
sanitizedPrefixBuffer []byte |
Otherwise LGTM |
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
What this PR does / why we need it:
This PR modifies the
/detected_fields
response to include the jsonpath of fields that were parsed from JSON objects. Since we use the_
character to represent both property nesting, as well as to escaped illegal characters from the label, there is no deterministic way to reverse engineer the jsonpath of the property from the label. Knowing the property is required to support a new JSON view for Logs Drilldown that will let users drilldown into certain JSON property, as we will be re-writing the query with arguments to the json parser to change the root key to the property the user has chosen to drill into, and we will need to know the correct path to that key.Benchmarks:
Which issue(s) this PR fixes:
Fixes #16816
Special notes for your reviewer:
Checklist
CONTRIBUTING.md
guide (required)feat
PRs 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.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR