Skip to content

Conversation

@alephys26
Copy link
Contributor

@alephys26 alephys26 commented Feb 10, 2026

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:

  • Added a telemetry.Method instance (executeRequestMethod) in client.go to track requests, successes, errors, and latency for the executeRequest and executeBatchRequest methods.
  • Updated executeRequest to increment request count, record latency, and track success or error outcomes using the new telemetry method. [1] [2] [3]
  • Updated executeBatchRequest to increment request count and record latency, with a "batch" label for batch operations.

Telemetry instrumentation for Trino SQL parser:

  • Added a telemetry.Method instance (parseAccessMethod) in parser.go to track SQL parsing requests and latency.
  • Updated the ParseAccess method to increment request count and record latency for each parse operation.

Minor code cleanup and import reordering:

  • Fixed import order in trino/listener.go for consistency.
  • Removed unnecessary blank lines in ranger.go and sql.go. [1] [2]

Testing

Tested locally:
image

Metric: go_app_telemetry.heimdall.ranger_execute_request.request.batch, Value: 1
Metric: go_app_telemetry.heimdall.ranger_execute_request.request, Value: 1
Observe Metric: go_app_telemetry.heimdall.ranger_execute_request.latency, Value: 1.000000
2026-02-10 16:15:14 - Pulled batch with 1 items...
Observe Metric: go_app_telemetry.heimdall.ranger_execute_request.latency.batch, Value: 1.000000
2026/02/10 16:15:14 Number of Ranger Users pulled: 1
2026/02/10 16:15:14 GetUsers returned users: map[debug-user:0x140001508c0]
Metric: go_app_telemetry.heimdall.trino_parse_access.request, Value: 1
line 1:22 mismatched input 'table' expecting {'ABSENT', 'ADD', 'ADMIN', 'AFTER', 'ALL', 'ANALYZE', 'ANY', 'ARRAY', 'ASC', 'AT', 'AUTHORIZATION', 'BEGIN', 'BERNOULLI', 'BOTH', 'CALL', 'CALLED', 'CASCADE', 'CATALOG', 'CATALOGS', 'COLUMN', 'COLUMNS', 'COMMENT', 'COMMIT', 'COMMITTED', 'CONDITIONAL', 'COUNT', 'COPARTITION', 'CURRENT', 'DATA', 'DATE', 'DAY', 'DECLARE', 'DEFAULT', 'DEFINE', 'DEFINER', 'DENY', 'DESC', 'DESCRIPTOR', 'DETERMINISTIC', 'DISTRIBUTED', 'DO', 'DOUBLE', 'EMPTY', 'ELSEIF', 'ENCODING', 'ERROR', 'EXCLUDING', 'EXPLAIN', 'FETCH', 'FILTER', 'FINAL', 'FIRST', 'FOLLOWING', 'FORMAT', 'FUNCTION', 'FUNCTIONS', 'GRACE', 'GRANT', 'GRANTED', 'GRANTS', 'GRAPHVIZ', 'GROUPS', 'HOUR', 'IF', 'IGNORE', 'IMMEDIATE', 'INCLUDING', 'INITIAL', 'INPUT', 'INTERVAL', 'INVOKER', 'IO', 'ISOLATION', 'ITERATE', 'JSON', 'KEEP', 'KEY', 'KEYS', 'LANGUAGE', 'LAST', 'LATERAL', 'LEADING', 'LEAVE', 'LEVEL', 'LIMIT', 'LOCAL', 'LOGICAL', 'LOOP', 'MAP', 'MATCH', 'MATCHED', 'MATCHES', 'MATCH_RECOGNIZE', 'MATERIALIZED', 'MEASURES', 'MERGE', 'MINUTE', 'MONTH', 'NESTED', 'NEXT', 'NFC', 'NFD', 'NFKC', 'NFKD', 'NO', 'NONE', 'NULLIF', 'NULLS', 'OBJECT', 'OF', 'OFFSET', 'OMIT', 'ONE', 'ONLY', 'OPTION', 'ORDINALITY', 'OUTPUT', 'OVER', 'OVERFLOW', 'PARTITION', 'PARTITIONS', 'PASSING', 'PAST', 'PATH', 'PATTERN', 'PER', 'PERIOD', 'PERMUTE', 'PLAN', 'POSITION', 'PRECEDING', 'PRECISION', 'PRIVILEGES', 'PROPERTIES', 'PRUNE', 'QUOTES', 'RANGE', 'READ', 'REFRESH', 'RENAME', 'REPEAT', 'REPEATABLE', 'REPLACE', 'RESET', 'RESPECT', 'RESTRICT', 'RETURN', 'RETURNING', 'RETURNS', 'REVOKE', 'ROLE', 'ROLES', 'ROLLBACK', 'ROW', 'ROWS', 'RUNNING', 'SCALAR', 'SCHEMA', 'SCHEMAS', 'SECOND', 'SECURITY', 'SEEK', 'SERIALIZABLE', 'SESSION', 'SET', 'SETS', 'SHOW', 'SOME', 'START', 'STATS', 'SUBSET', 'SUBSTRING', 'SYSTEM', 'TABLES', 'TABLESAMPLE', 'TEXT', 'STRING', 'TIES', 'TIME', 'TIMESTAMP', 'TO', 'TRAILING', 'TRANSACTION', 'TRUNCATE', 'TRY_CAST', 'TYPE', 'UNBOUNDED', 'UNCOMMITTED', 'UNCONDITIONAL', 'UNIQUE', 'UNKNOWN', 'UNMATCHED', 'UNTIL', 'UPDATE', 'USE', 'USER', 'UTF16', 'UTF32', 'UTF8', 'VALIDATE', 'VALUE', 'VERBOSE', 'VERSION', 'VIEW', 'WHILE', 'WINDOW', 'WITHIN', 'WITHOUT', 'WORK', 'WRAPPER', 'WRITE', 'YEAR', 'ZONE', IDENTIFIER_, DIGIT_IDENTIFIER_, QUOTED_IDENTIFIER_, BACKQUOTED_IDENTIFIER_}
Observe Metric: go_app_telemetry.heimdall.trino_parse_access.latency, Value: 5.000000
2026/02/10 16:15:14 ParseAccess executed

Copilot AI review requested due to automatic review settings February 10, 2026 09:09
sanketjadhavSF
sanketjadhavSF previously approved these changes Feb 10, 2026
Copy link
Contributor

Copilot AI left a 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.Method counters and latency tracking.
  • Instrumented Trino ParseAccess with 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 capacity pageSize (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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants