Skip to content

feat: Add BigQuery Agent Analytics Plugin#738

Open
vasilyl wants to merge 1 commit intogoogle:mainfrom
vasilyl:main
Open

feat: Add BigQuery Agent Analytics Plugin#738
vasilyl wants to merge 1 commit intogoogle:mainfrom
vasilyl:main

Conversation

@vasilyl
Copy link
Copy Markdown

@vasilyl vasilyl commented Apr 17, 2026

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

2. Or, if no issue exists, describe the change:

Problem:
No Go version of the ADK BigQuery Agent Analytics Plugin

Solution:
Port the BigQuery Agent Analytics plugin available in Python and Java to Go ADK.

Testing Plan

Tests are included for the main plugin module bigquery_agent_analytics_plugin.go as well as for the batch_processor.go library and json_formatter.go utility.

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

The tests inject fake HTTP and GRPC clients which mock interactions with BigQuery API.

Manual End-to-End (E2E) Tests:

Usage example

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

@vasilyl vasilyl marked this pull request as draft April 17, 2026 21:55
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a BigQuery Agent Analytics plugin for the ADK, featuring an asynchronous batch processor that uses Apache Arrow for efficient event logging. The implementation includes comprehensive callbacks for tracking agent, model, and tool interactions, along with configurable truncation logic for large payloads. Feedback suggests optimizing the flush mechanism to reduce goroutine churn, addressing a potential schema mismatch where nulls could be appended to a non-nullable timestamp field, and adding safeguards against infinite recursion in the JSON truncation utility.

Comment thread plugin/agentanalytics/batch_processor.go
Comment thread plugin/agentanalytics/batch_processor.go
Comment thread plugin/agentanalytics/json_formatter.go
@vasilyl vasilyl marked this pull request as ready for review April 21, 2026 19:45
@caohy1988
Copy link
Copy Markdown

Review: BigQuery Agent Analytics Plugin — Go Port

Thorough review against the Python reference implementation on adk-python main. Findings organized by severity.


High

H1. Missing sensitive key redaction (security)
json_formatter.gorecursiveSmartTruncate

The Python version redacts _SENSITIVE_KEYS (client_secret, access_token, refresh_token, id_token, api_key, password) and all temp:-prefixed keys before writing to BQ. The Go port does no key-based redaction. OAuth tokens and API keys will be written to BigQuery in plaintext.

H2. No circular reference detection
json_formatter.gorecursiveSmartTruncate

The Python version tracks seen object IDs and returns "[CIRCULAR_REFERENCE]" for cycles. The Go port traverses map[string]any values via reflection with no cycle guard. A self-referencing map would cause infinite recursion and a stack overflow panic.

H3. Race condition on BatchProcessor.started
batch_processor.goStart() / Close()

started is a plain bool read/written without synchronization. If Start() and Close() are called from different goroutines, this is a data race. Should use sync.Once for startup or atomic.Bool.

H4. Mutex held during retry sleeps
batch_processor.gostreamWriter.append

s.mu.Lock() is acquired at the top and held for the entire retry loop, including time.After(delay) sleeps (up to 20+ seconds with default config). During this time, all other flushes block — the ticker-based flush and blockingFlush during shutdown cannot make progress. The Python version uses async I/O and doesn't have this blocking problem. Release the lock between retries or restructure the retry to not hold the mutex during waits.


Medium

M1. Event type names don't match Python
bigquery_agent_analytics_plugin.go

Go Python
USER_MESSAGE USER_MESSAGE_RECEIVED
INVOCATION_START INVOCATION_STARTING
INVOCATION_END INVOCATION_COMPLETED
AGENT_START / AGENT_END AGENT_STARTING / AGENT_COMPLETED
MODEL_REQUEST / MODEL_RESPONSE LLM_REQUEST / LLM_RESPONSE
MODEL_ERROR LLM_ERROR
TOOL_START / TOOL_END TOOL_STARTING / TOOL_COMPLETED

If both SDKs write to the same BQ table, queries, dashboards, and the Python-side _EVENT_VIEW_DEFS views will not find Go-produced events. This should match the Python naming to maintain cross-language compatibility.

M2. Missing event types
bigquery_agent_analytics_plugin.go

The Python version logs STATE_DELTA, A2A_INTERACTION, HITL_CREDENTIAL_REQUEST, HITL_CONFIRMATION_REQUEST, HITL_INPUT_REQUEST, and their _COMPLETED variants. The Go port has a generic EVENT callback that doesn't decompose into specialized types.

M3. SerializedArrowSchema may produce wrong format
schema.go

ipc.NewWriter + Close() produces a full IPC stream (header + end-of-stream marker). The BQ Storage Write API's WriterSchema.ArrowData expects the serialized Arrow schema message only. The Python version uses pyarrow.ipc.serialize_schema() which outputs only the schema flatbuffer bytes. This could cause schema parsing errors on the BQ server side.

M4. AfterRunCallback flush can silently skip
bigquery_agent_analytics_plugin.go

processor.flush() uses TryLock — if a background flush holds the lock, the "synchronous" post-invocation flush silently does nothing. Events from the final invocation may not reach BQ until the next periodic tick or shutdown.

M5. Unbounded goroutine creation in Append
batch_processor.go

if len(b.queue) == b.config.BatchSize {
    go b.flush()
}

len(b.queue) is racy after the channel send, and each trigger spawns a new goroutine. Under burst conditions many goroutines pile up, all competing for flushMu.TryLock(). Consider using a single flush signal channel instead.

M6. Shutdown flush loop has no timeout check
batch_processor.go

for len(b.queue) > 0 {
    b.blockingFlush(shutdownCtx)
}

If blockingFlush fails without draining the queue, this spins until shutdownCtx expires only indirectly (via retry timeouts within each call). The loop should check shutdownCtx.Err() explicitly.


Low

L1. LogMultiModalContent config flag is ignored
bigquery_agent_analytics_plugin.go

Config has LogMultiModalContent bool but logEvent always logs content_parts regardless. The Python version conditionally includes content_parts based on this setting.

L2. parent_span_id is never populated
bigquery_agent_analytics_plugin.go

The schema includes parent_span_id but the Go code never sets it. The Python version extracts it from the span context/plugin stack.

L3. FormatContentParts returns nil but tests expect empty slice
json_formatter.go / json_formatter_test.go

var parts []map[string]any initializes to nil, but the test expects []map[string]any{}. These are not equal under reflect.DeepEqual. The nil-content and empty-parts tests will fail.

L4. config.CustomTags map shared without defensive copy
bigquery_agent_analytics_plugin.go

Config is passed by value, but CustomTags map[string]string is a reference type. If the caller mutates the map after plugin creation, it's a data race during logEvent. Copy the map in the constructor.

L5. UnexportedField in test struct is actually exported
json_formatter_test.go

UnexportedField string starts with uppercase — it IS exported in Go. The name is misleading; if testing unexported-field exclusion, use unexportedField.

L6. Unrelated dependency bumps
go.mod

modernc.org/sqlite jumps from v1.21.1 to v1.46.1, modernc.org/libc from v1.22.3 to v1.67.6. These may have been pulled transitively but are very large jumps that could affect the SQLite session backend.


Summary

Priority Count Key items
Must fix 4 Credential redaction, circular ref panic, started race, mutex-during-retry
Should fix 6 Event type naming parity, missing event types, Arrow schema format, flush reliability
Nice to fix 6 Config flag ignored, parent_span_id empty, test bugs, dep bumps

The overall structure is solid — the batch processor, config, schema, and plugin lifecycle are well-designed. The main gaps are security (H1), concurrency safety (H3/H4), and cross-language schema parity (M1/M2).

@caohy1988
Copy link
Copy Markdown

Review finding:

Medium: plugin initialization swallows table-setup failures and can return a "working" plugin that will only fail later at write time. In plugin/agentanalytics/bigquery_agent_analytics_plugin.go:96-113, any tableRef.Metadata(ctx) error is treated as "table not found", then Create(...) errors are only logged and ignored. That means permission errors, transient BigQuery failures, or a failed table create still allow construction to continue into NewBatchProcessor(...), so misconfiguration degrades into silent runtime event loss instead of an init-time error.

Recommended fix: distinguish NotFound from other metadata errors, and if table creation fails, return the error from NewBigQueryAgentAnalyticsPluginWithClients instead of just logging it.

@vasilyl vasilyl marked this pull request as draft April 23, 2026 21:07
@vasilyl
Copy link
Copy Markdown
Author

vasilyl commented Apr 24, 2026

L6. Unrelated dependency bumps
go.mod

modernc.org/sqlite jumps from v1.21.1 to v1.46.1, modernc.org/libc from v1.22.3 to v1.67.6. These may have been pulled transitively but are very large jumps that could affect the SQLite session backend.

The modernc.org/sqlite and modernc.org/libc bumps are not direct, manual changes.
They are transitive requirements brought in by github.com/apache/arrow-go/v18@v18.5.2.

go mod graph shows:

  • google.golang.org/adk -> github.com/apache/arrow-go/v18@v18.5.2 -> modernc.org/sqlite@v1.46.1
  • google.golang.org/adk -> github.com/apache/arrow-go/v18@v18.5.2 -> modernc.org/libc@v1.67.6

go mod tidy did not remove those indirect entries, so they are currently required by the module graph.

@vasilyl vasilyl marked this pull request as ready for review April 24, 2026 04:19
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.

2 participants