-
Notifications
You must be signed in to change notification settings - Fork 6
Add metrics to Trino SQL Parser and Ranger Client #91
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
Adds telemetry instrumentation to Ranger client HTTP request execution and Trino SQL access parsing to improve observability (request counts and latency), plus minor formatting/import cleanups.
Changes:
- Instrumented Ranger client request execution with
telemetry.Methodcounters and latency tracking. - Instrumented Trino
ParseAccesswith request counting and latency tracking. - Minor gofmt/import-order/whitespace-only cleanups in RBAC, SQL parser, and Ranger tests.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/rbac/rbac.go | Whitespace/formatting-only change. |
| internal/pkg/sql/parser/trino/parser.go | Adds telemetry to ParseAccess (request count + latency). |
| internal/pkg/sql/parser/trino/listener.go | Import order adjustment. |
| internal/pkg/sql/parser/sql.go | Removes stray blank line / formatting. |
| internal/pkg/rbac/ranger/tests/ranger_policy_check_test.go | Formatting-only (alignment/spacing). |
| internal/pkg/rbac/ranger/tests/group_policy_test.go | Formatting-only (alignment/spacing). |
| internal/pkg/rbac/ranger/ranger.go | Removes unnecessary blank line. |
| internal/pkg/rbac/ranger/client.go | Adds telemetry method + instrumentation to request/batch execution. |
Comments suppressed due to low confidence (1)
internal/pkg/rbac/ranger/client.go:193
results := make([]getResponse, 500)initializes the slice with length 500, and then the code appends batches to it. This causes the returned slice to contain 500 zero-value entries before the real results, which then forces callers to iterate lots of empties and inflates memory usage. Initialize with length 0 and capacitypageSize(or similar) instead.
results := make([]getResponse, 500)
pageSize := 500
startIndex := 0
for {
batchEndpoint := fmt.Sprintf("%s?pageSize=%d&startIndex=%d", endpoint, pageSize, startIndex)
// Marshall into generic get
getResponse := &getResponse{}
if err := c.executeRequest(method, batchEndpoint, getResponse, nil); err != nil {
return nil, err
}
// Add this batch's response to our results
results = append(results, *getResponse)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…erninc/heimdall into alephys26/add-parser-ranger-metrics
This pull request adds telemetry instrumentation to the Ranger client and Trino SQL parser components to improve observability of request and parsing operations. The main changes introduce telemetry counters and latency recording for key methods, allowing for better monitoring of request rates, successes, errors, and latencies.
Telemetry instrumentation for Ranger client:
telemetry.Methodinstance (executeRequestMethod) inclient.goto track requests, successes, errors, and latency for theexecuteRequestandexecuteBatchRequestmethods.executeRequestto increment request count, record latency, and track success or error outcomes using the new telemetry method. [1] [2] [3]executeBatchRequestto increment request count and record latency, with a "batch" label for batch operations.Telemetry instrumentation for Trino SQL parser:
telemetry.Methodinstance (parseAccessMethod) inparser.goto track SQL parsing requests and latency.ParseAccessmethod to increment request count and record latency for each parse operation.Minor code cleanup and import reordering:
trino/listener.gofor consistency.ranger.goandsql.go. [1] [2]Testing
Tested locally:
