feat(agw): add implicit auditlog usage on agw by ricardosrib · Pull Request #209 · SAP/cloud-sdk-python

ricardosrib · 2026-07-06T18:18:54Z

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Implicit audit log emission in AgentGatewayClient - the client now automatically creates an AuditClient on initialization (using the LoB destination) and sends a DataAccess audit event after every successful list_mcp_tools() and call_mcp_tool() call. The tenant ID is read from the telemetry context var (get_tenant_id()).

Related Issue

N/A

Type of Change

Please check the relevant option:

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Code refactoring
Dependency update

How to Test

Create a AgentGatewayClient with a valid tenant_subdomain in an environment where the audit log destination is configured
Call list_mcp_tools() or call_mcp_tool() with a request context that has a tenant ID set
Verify a DataAccess audit event is emitted with channel_type="MCP", channel_id="agent-gateway", object_type="mcp-tool", and the correct tenant_id and object_id
For customer agents (no tenant_subdomain): verify no audit event is emitted and no error is raised
Run unit tests: uv run pytest tests/agentgateway/unit/test_agw_client.py

Checklist

Before submitting your PR, please review and check the following:

I have read the Contributing Guidelines
I have verified that my changes solve the issue
I have added/updated automated tests to cover my changes
All tests pass locally
I have verified that my code follows the Code Guidelines
I have updated documentation (if applicable)
I have added type hints for all public APIs
My code does not contain sensitive information (credentials, tokens, etc.)
I have followed Conventional Commits for commit messages

Breaking Changes

N/A

Additional Notes

N/A

BSD awk (macOS) requires the '/' inside a character class to be escaped as '\/'. GNU awk accepts either form. Discovered during live run on PR #209. Impact: check-testing-depth now returns valid JSON on macOS orchestrate runs; previously it silently emitted an awk error and aggregate.sh skipped the report.

Discovered during live run against PR #209. Multiple checks used the pattern: count=$(echo "$diff" | grep -E 'pattern' 2>/dev/null | wc -l | tr -d ' ') Under 'set -o pipefail', when grep finds zero matches it returns exit 1, which propagates through the pipe. Under 'set -e', the containing script exits silently — no stderr, no stdout, exit code 1. The 'aggregate.sh skips invalid JSON' safety net (added earlier) masked this by producing a summary that omitted the failing checks. Fix: wrap each grep-in-pipe with { grep … || true; } so zero-match returns success. Affects: - check-deps-supply.sh (count_lines helper) - check-pr-size.sh (count_lines helper + mods_count) - check-telemetry.sh (client_files, test_files, new_decorators) - check-testing-depth.sh (new_modules) - check-deletion-hygiene.sh (hits) Impact: live orchestrate.sh runs now emit valid JSON for all 20 checks instead of 15/20. No FP regression (all fixes preserve semantics). Discovered by: real dry-run on cloud-sdk-python PR #209 (Ricardo's agentgateway auditlog PR).

Discovered live on cloud-sdk-python PR #209 (Ricardo, agentgateway auditlog). The check was counting string occurrences from the on-disk AST — global to the file — then anchoring the finding to the first occurrence's line number. When a PR (a) shifts line numbers by adding upstream code and (b) does not touch any of the actual repeated-string lines, PY-CON-01 fires for tech debt that pre-existed the PR. Fix: read $ADDED_LINES_FILE (already exported by orchestrate.sh) inside check_con_01. Filter each literal's occurrence list to lines actually in the PR's added set. Only fire when ${filtered} is non-empty AND total occurrences (across file, unchanged) still >= threshold. Anchor line becomes the first added-line occurrence. Backward-compat: if ADDED_LINES_FILE is unset (bats tests exercising the lib directly), fall back to the legacy full-file behavior. Two new regression tests pin both branches (untouched pre-existing repetition = no fire; at least one occurrence on added line = fires with correct anchor). Bats: 81/81 green on both repos.

NicoleMGomes · 2026-07-09T17:16:12Z

+            logger.debug("Failed to create audit client", exc_info=True)
+            return None
+
+    def _send_audit_event(


Create a specific py file for audit log functions.

NicoleMGomes · 2026-07-09T17:16:53Z

Update user guide

NicoleMGomes · 2026-07-09T17:17:41Z

+            )
+        except Exception:
+            logger.debug("Failed to create audit client", exc_info=True)
+            return None


Maybe this should be a configuration, we need to create a pattern for implicit usage.

NicoleMGomes · 2026-07-09T17:18:22Z

        self,
        user_token: str | Callable[[], str] | None = None,
        app_tid: str | None = None,
+        user_id: str | None = None,


Not needed. Only run_mcp_tools needs to be audited in my opinion.

NicoleMGomes · 2026-07-09T17:18:39Z

                for tenant-scoped token exchange.
                TODO: This parameter's requirement is still being clarified with
                the IBD team and may be removed if unnecessary.
+            user_id: User identifier recorded in the audit event when an


tiagoek · 2026-07-09T17:40:17Z

+                tenant=tenant_subdomain,
+                _telemetry_source=Module.AGENTGATEWAY,
+            )
+        except Exception:


[FLAG] PY-EL-02

except block does not end with raise — swallowing? Add re-raise or comment justifying suppression

tiagoek · 2026-07-09T17:40:33Z

+            event.object_type = "mcp-tool"
+            event.object_id = object_id
+            self._audit_client.send(event)
+        except Exception:


[FLAG] PY-EL-02

except block does not end with raise — swallowing? Add re-raise or comment justifying suppression

tiagoek · 2026-07-09T17:40:48Z

SDK Module Review

Check	Status	Findings
bdd	✅ PASS	0
binding-shape	✅ PASS	0
commits	✅ PASS	0
concurrency	✅ PASS	0
constants	✅ PASS	0
deletion-hygiene	✅ PASS	0
deps-supply	✅ PASS	0
disclosure	✅ PASS	0
docs	✅ PASS	0
errors-logging	⚠️ FLAG	2
hardcode	✅ PASS	0
http-hygiene	✅ PASS	0
license-spdx	✅ PASS	0
patterns	✅ PASS	0
pr-size	✅ PASS	0
quality-gate-parity	✅ PASS	0
secrets	✅ PASS	0
telemetry	✅ PASS	0
testing-depth	✅ PASS	0
versioning	✅ PASS	0

Findings (2)

[FLAG] PY-EL-02 — except block does not end with raise — swallowing? Add re-raise or comment justifying suppression (inline on src/sap_cloud_sdk/agentgateway/agw_client.py:183)
[FLAG] PY-EL-02 — except block does not end with raise — swallowing? Add re-raise or comment justifying suppression (inline on src/sap_cloud_sdk/agentgateway/agw_client.py:207)

2 finding(s): 2 posted as inline comment(s) on the affected lines, 0 not tied to a code line (listed above).

Generated by sdk-review-skill · v1

ricardosrib added 3 commits July 6, 2026 12:00

feat: add implicit audit log usage on agent gateway

bb614bd

create audit client on initialization time of the agw client

fa3c770

update unit tests

bd60cd3

ricardosrib requested a review from a team as a code owner July 6, 2026 18:18

ricardosrib changed the title ~~feat: add implicit auditlog usage on agw~~ feat(agw): add implicit auditlog usage on agw Jul 6, 2026

ricardosrib and others added 3 commits July 6, 2026 15:38

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

74d50c6

version bump

f99f6b4

resolve pre-commit issue

5515bea

ricardosrib force-pushed the feat/add-implicit-auditlog-on-agw branch from a9e31e7 to 5515bea Compare July 6, 2026 19:02

ricardosrib and others added 3 commits July 7, 2026 15:15

move import to the top

8d5b08e

ruff format

02dc447

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

5550854

tiagoek added the sdk-review: ⚠️ flagged label Jul 8, 2026

tiagoek removed the sdk-review: ⚠️ flagged label Jul 8, 2026

tiagoek added sdk-review: ❌ blocked sdk-review: ⚠️ flagged and removed sdk-review: ❌ blocked sdk-review: ⚠️ flagged labels Jul 8, 2026

NicoleMGomes reviewed Jul 9, 2026

View reviewed changes

NicoleMGomes requested changes Jul 9, 2026

View reviewed changes

tiagoek reviewed Jul 9, 2026

View reviewed changes

tiagoek added sdk-review: ⚠️ flagged and removed sdk-review: ⚠️ flagged labels Jul 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(agw): add implicit auditlog usage on agw#209

feat(agw): add implicit auditlog usage on agw#209
ricardosrib wants to merge 9 commits into
mainfrom
feat/add-implicit-auditlog-on-agw

ricardosrib commented Jul 6, 2026

Uh oh!

NicoleMGomes Jul 9, 2026

Uh oh!

NicoleMGomes commented Jul 9, 2026

Uh oh!

NicoleMGomes Jul 9, 2026

Uh oh!

NicoleMGomes Jul 9, 2026

Uh oh!

NicoleMGomes Jul 9, 2026

Uh oh!

tiagoek Jul 9, 2026

Uh oh!

tiagoek Jul 9, 2026

Uh oh!

tiagoek commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ricardosrib commented Jul 6, 2026

Description

Related Issue

Type of Change

How to Test

Checklist

Breaking Changes

Additional Notes

Uh oh!

NicoleMGomes Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes commented Jul 9, 2026

Uh oh!

NicoleMGomes Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

NicoleMGomes Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

tiagoek Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

tiagoek Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

tiagoek commented Jul 9, 2026

SDK Module Review

Findings (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants