ADK Agent Container Image - #6894
Conversation
| @@ -0,0 +1,56 @@ | |||
| # ADK Agent Container | |||
|
|
|||
| FastAPI service deployed inside the GKE cluster that serves as the | |||
| sandbox-side metrics. | ||
| """ | ||
| async with _benchmark_lock: | ||
| os.environ["BENCHMARK_MODE"] = "density" |
There was a problem hiding this comment.
Not urgent, but maybe for a followup PR:
Using os env to pass configs is not very intuitive, and also has the risk of race conditions. Consider using the prompt to pass the configs
POST /benchmark/python/density
|
v
FastAPI service
|
| (prompt: {mode: "density", sample_count: 100...})
v
ADK agent
|
| (prompt: {mode: "density", sample_count: 100...})
v
MockLLM
|
| (script instruction based on the prompt)
v
ADK agent (execution)
There was a problem hiding this comment.
@roycaihw Using os.environ is a design choice to keep the Python benchmark scripts static. If we passed parameters via the prompt, our MockLlm would have to perform string-replacement on the Python script's source code to inject the variables (e.g., changing SAMPLE_COUNT = 20 to 100) before sending it to the sandbox. With os.environ, the Python script remains static, and we cleanly inject the variables into the sandbox via standard Linux environment variables (/bin/sh -c 'SAMPLE_COUNT=100 python3 script.py').
Regarding race conditions: we explicitly use an asyncio.Lock() (_benchmark_lock) around the endpoint handlers to serialize benchmark requests, ensuring concurrent POSTs cannot clobber the shared environment variables.
| req.concurrent_sessions, | ||
| ) | ||
|
|
||
| prompt = "Please start the GKE performance benchmark workflow." |
There was a problem hiding this comment.
The prompt is not used, correct? Can we drop it?
There was a problem hiding this comment.
You are correct that the content of the prompt is ignored by our MockLlm. However, the ADK Runner.run_async() method requires a non-empty string to initiate the conversation loop and trigger the first LLM generation. I can simplify the string to just "start" to make it clear that it is simply a trigger, but we cannot drop the variable entirely.
| ) | ||
|
|
||
|
|
||
| class ChromiumBenchmarkRequest(BaseModel): |
There was a problem hiding this comment.
Right now the main.py combines all the endpoints and their Request class. For better readability, can you split each endpoint and its Request class into a separate file?
There was a problem hiding this comment.
I'll refactor into a "modular" FastAPI structure with an api/routes/ directory, moving each endpoint and its models into separate files.
| _benchmark_lock = asyncio.Lock() | ||
|
|
||
|
|
||
| def _percentile_stats(sorted_values: list, prefix: str) -> dict: |
There was a problem hiding this comment.
Can you turn this function into a util for reuse? You have two methods (_percentile_stats and _percentile). Make them consistent.
There was a problem hiding this comment.
I'll consolidate the percentile logic into a shared api/utils.py file, sure.
| logging.exception("Error shutting down ThreadPoolExecutor") | ||
|
|
||
|
|
||
| app = FastAPI(title="GKE Benchmark Agent", version="0.2.0", lifespan=lifespan) |
| POST /benchmark/chromium/density → run the Chromium density benchmark | ||
| POST /run → raw ADK agent interaction | ||
|
|
||
| POST /benchmark/python/density — Request: |
There was a problem hiding this comment.
The documentation seems incomplete as it only has the request and response for POST /benchmark/python/density. I'd recommend putting the API endpoint documentation in a separate doc file.
There was a problem hiding this comment.
I'll move the API doc out of main.py and into a new API_DOCS.md file.
| elapsed_ms = (time.perf_counter() - t0) * 1000 | ||
|
|
||
| print(json.dumps({ | ||
| "sandbox_status": "ok", |
There was a problem hiding this comment.
Let's make this a "success" v.s. "fail" based on whether result matches the expectation (49,995,000-- the sum of all integers from 0 up to 9,999)
| print(json.dumps({ | ||
| "sandbox_status": "ok", | ||
| "sandbox_qps_exec_ms": round(elapsed_ms, 3), | ||
| "sandbox_compute_result": result, |
There was a problem hiding this comment.
Let's drop the "sandbox_compute_result" since it's just confusing to people unless they read the actual source of what the sandbox does.
|
|
||
| print(json.dumps({ | ||
| "sandbox_status": "ok", | ||
| "sandbox_qps_exec_ms": round(elapsed_ms, 3), |
There was a problem hiding this comment.
Let's drop "qps" from the metrics name since this metrics measures how long a simple python sandbox execution takes-- nothing related to qps here.
|
Thanks for the review Roy. Give me a ping when Roy's lgtm'ed & I'll add the lgtm + tag which will start the merging process |
3 participants
Files: ~13 new files (no Python benchmark logic)
perfkitbenchmarker/data/docker/agentic/adk-agent/README.mdperfkitbenchmarker/data/docker/agentic/adk-agent/Dockerfileperfkitbenchmarker/data/docker/agentic/adk-agent/__init__.pyperfkitbenchmarker/data/docker/agentic/adk-agent/.dockerignoreperfkitbenchmarker/data/docker/agentic/adk-agent/.gcloudignoreperfkitbenchmarker/data/docker/agentic/adk-agent/cloudbuild-arm64.yamlperfkitbenchmarker/data/docker/agentic/adk-agent/requirements.txtperfkitbenchmarker/data/docker/agentic/adk-agent/main.pyperfkitbenchmarker/data/docker/agentic/adk-agent/gke_performance_agent/__init__.pyperfkitbenchmarker/data/docker/agentic/adk-agent/gke_performance_agent/agent.pyperfkitbenchmarker/data/docker/agentic/adk-agent/sandboxed_apps/python_test_app/benchmark_density.pyperfkitbenchmarker/data/docker/agentic/adk-agent/sandboxed_apps/python_test_app/benchmark_payload.pyperfkitbenchmarker/data/docker/agentic/adk-agent/sandboxed_apps/python_test_app/benchmark_qps.pyDescription: Adds the ADK Agent container image — a FastAPI service deployed inside the GKE cluster that the PKB benchmark modules interact with via HTTP. Includes:
BenchmarkGkeCodeExecutorNothing in PKB imports these files — they are copied into a container image at build time.