[LucasAlvesSoares]

Description

Adds the sap.qos.class quality-of-service attribute, hardcoded to at_least_once, to every audit log record emitted by the auditlog_ng client.

Changes

• src/sap_cloud_sdk/core/telemetry/constants.py — added ATTR_SAP_QOS_CLASS = "sap.qos.class" and QOS_CLASS_AT_LEAST_ONCE = "at_least_once".
• src/sap_cloud_sdk/core/auditlog_ng/client.py — emit the QoS attribute on every audit LogRecord in AuditClient.send().
• tests/core/unit/auditlog_ng/unit/test_client.py — assert the attribute is set to at_least_once on emitted records.

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

1. Run the audit log client unit tests: python -m pytest tests/core/unit/auditlog_ng/unit/test_client.py
2. Inspect the attributes emitted by AuditClient.send() / send_json() (e.g. via the test's mocked logger emit call args).
3. Expected result: every emitted LogRecord includes the attribute sap.qos.class with the value at_least_once, in addition to the existing cloudevents.event_id, sap.tenancy.tenant_id, and sap.auditlogging.mime_type attributes.

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
• (New modules only) I have attached e2e evidence showing the module working against a real BTP service
• (Security-sensitive changes only) I have run /security-review and addressed all ❌ findings

Additional Notes

• The QoS attribute value is intentionally hardcoded to at_least_once for all audit logs; it is not configurable.

[LucasAlvesSoares]

PR Review: #426: feat: introduce sap quality of service attribute to audit logs

Author: LucasAlvesSoares Branch: qos-for-auditlog → main
Verdict: ⚠️ Needs Minor Work
Summary: Small, well-scoped change adding a hardcoded sap.qos.class="at_least_once" attribute to every emitted audit log record; code is clean and tested, but the JSON path lacks an explicit test assertion and one CI gate (the review label) is still pending.

---

A: Process & Compliance

#	Criterion	Status	Finding
A1	PR template complete	⚠️	Description, Type of Change, How to Test, and core checklist all present/ticked. "Related Issue" section was removed rather than filled — acceptable since no issue is tracked, but see A3. Not a new module and not security-sensitive, so the two conditional items correctly left unticked.
A2	Conventional Commits	✅	PR title feat: introduce sap quality of service attribute to audit logs is valid. Commit Validation CI job passed.
A3	Issue linked	⚠️	No Closes #N in the body. Non-blocking.
A4	AI-generated code disclosure	➖	Change is trivial/mechanical; no disclosure required.

B: Security & Sensitive Data

#	Criterion	Status	Finding
B1	No sensitive data in code	✅	Only semantic-convention string constants (sap.qos.class, at_least_once).
B2	No sensitive data in PR body	✅	No internal URLs, tenant IDs, or customer names.

C: Code Quality

#	Criterion	Status	Finding
C1	CI checks passing	⚠️	Code Quality, Tests & Coverage, Build, CodeQL, Commit Validation, Version Bump all pass. Integration Tests fails on tests/agentgateway/integration/test_agw_bdd.py::test_get_ias_client_id_returns_a_nonempty_string (OAuth2 "Missing access token") — a pre-existing, environment-dependent agentgateway failure unrelated to this change. SDK review label present fails pending this review.
C2	Version bump	➖	No manual pyproject.toml version change.
C3	Type hints	✅	No public signatures changed; new symbols are module-level string constants.
C4	No hardcoded values	✅	Attribute key and value extracted to constants ATTR_SAP_QOS_CLASS / QOS_CLASS_AT_LEAST_ONCE in core/telemetry/constants.py:30-31, consistent with the existing ATTR_SAP_* convention.
C5	Import organization	✅	Top-level grouped import from telemetry.constants in client.py:41-44.
C6	Naming conventions	✅	ATTR_SAP_QOS_CLASS matches the ATTR_SAP_* family; value constant is SCREAMING_SNAKE_CASE.
C7	No unused code	✅	Both imported constants are used (client.py:256; test assertion).
C8	No unjustified new dependencies	➖	No dependency changes.
C9	Proto code freshness	➖	No .proto changes.

D: API & Design

#	Criterion	Status	Finding
D1	API future-proofing	✅	No new public API surface; attribute emitted internally in AuditClient.send().
D2	Public API hygiene	✅	New constants live in the internal telemetry constants module; nothing added to a public __all__.
D3	Breaking changes marked	✅	Purely additive — a new attribute on emitted records; no removed/renamed symbols.
D4	Pagination & tenant filtering	➖	No list/query operation.
D5	Telemetry instrumentation	✅	This change is telemetry; send() already routes through existing instrumentation and the attribute is placed alongside existing OTel attributes.
D6	Multi-tenancy support	➖	No change to tenancy behavior.

E: Tests & Documentation

#	Criterion	Status	Finding
E1	Tests added/updated	⚠️	test_send_binary_success asserts attributes[ATTR_SAP_QOS_CLASS] == QOS_CLASS_AT_LEAST_ONCE. Good. However test_send_json_success was not extended with the same assertion — the JSON path (send_json → send) shares the attribute dict, so it is covered transitively, but an explicit assertion would guard against future divergence.
E2	Documentation quality	⚠️	The auditlog_ng/user-guide.md is not updated to mention the new sap.qos.class attribute. Minor — the attribute is non-configurable and transparent to callers, so this is optional.
E2a	Multi-tenancy documentation	➖	user-guide.md already contains a ## Multi-tenancy section and is unchanged by this PR.
E3	Module structure compliance	➖	No new module.

---

❌ Blocking Issues

• None.

⚠️ Non-Blocking Suggestions

• [E1]: Add assert kwargs["attributes"][ATTR_SAP_QOS_CLASS] == QOS_CLASS_AT_LEAST_ONCE to test_send_json_success so both serialization paths explicitly assert the QoS attribute.
• [C1]: Confirm the Integration Tests failure is the pre-existing agentgateway OAuth2 flake and not introduced here (the failing test is unrelated to auditlog). Re-run once the environment token is available.
• [A3]: Add Closes #N if a tracking issue exists.
• [E2]: Optionally note the always-on sap.qos.class="at_least_once" attribute in auditlog_ng/user-guide.md.
• Style (nit): The sibling literals "sap.tenancy.tenant_id" and "cloudevents.event_id" in the same emit() call remain inline even though ATTR_SAP_TENANT_ID already exists in the constants file. Not introduced by this PR, but the new line makes the inconsistency visible — consider switching the neighbors to the shared constant in a follow-up.

✅ Things Done Well

• Attribute key and value correctly extracted to shared constants rather than inline literals, matching the established ATTR_SAP_* convention and the correct semantic-convention value (at_least_once, not best_effort).
• Constants placed in core/telemetry/constants.py — the right home, already owning sibling SAP attributes; the auditlog module already depends on the telemetry package.
• Minimal, focused diff (3 files, +18/−1) with a test assertion added.

[LucasAlvesSoares]

PR Review: #426: feat: introduce sap quality of service attribute to audit logs

Author: LucasAlvesSoares Branch: qos-for-auditlog → main
Verdict: ✅ Ready to Merge
Summary: Clean, additive change adding a hardcoded sap.qos.class="at_least_once" attribute to every emitted audit log record; the two ⚠️ items from the prior review (JSON-path test assertion and user-guide note) are now resolved in the follow-up docs and tests commit.

Re-review of head d4f29f6 (2nd commit docs and tests). Both previously-flagged ⚠️ findings (E1, E2) are addressed; nothing new introduced. Diff is now 4 files, +22/−1.

---

A: Process & Compliance

#	Criterion	Status	Finding
A1	PR template complete	⚠️	Description, Type of Change, How to Test, and core checklist present/ticked. "Related Issue" omitted (no tracked issue). The "updated documentation" checkbox is still unticked even though the user-guide was updated in this PR — tick it for accuracy. Conditional items (new-module e2e, security-review) correctly left unticked.
A2	Conventional Commits	✅	PR title feat: introduce sap quality of service attribute to audit logs is valid; Commit Validation passed. (PRs squash-merge, so the per-commit docs and tests headline is not evaluated.)
A3	Issue linked	⚠️	No Closes #N in the body. Non-blocking.
A4	AI-generated code disclosure	➖	Trivial/mechanical change; no disclosure required.

B: Security & Sensitive Data

#	Criterion	Status	Finding
B1	No sensitive data in code	✅	Only semantic-convention string constants (sap.qos.class, at_least_once).
B2	No sensitive data in PR body	✅	No internal URLs, tenant IDs, or customer names.

C: Code Quality

#	Criterion	Status	Finding
C1	CI checks passing	⚠️	SDK review label present, Build & Package, Check Version Bump, Commit Validation, REUSE, CLA all pass. Code Quality Checks, Tests & Coverage, Analyze (python), Integration Tests are pending (re-triggered by the 2nd push). Expected to go green; the integration suite's only prior failure was the unrelated agentgateway OAuth2 flake. Not blocking merge pending re-run.
C2	Version bump	➖	No manual pyproject.toml version change.
C3	Type hints	✅	No public signatures changed; new symbols are module-level string constants.
C4	No hardcoded values	✅	Key/value extracted to ATTR_SAP_QOS_CLASS / QOS_CLASS_AT_LEAST_ONCE in core/telemetry/constants.py:30-31.
C5	Import organization	✅	Top-level grouped import from telemetry.constants in client.py:41-44; test imports at test_client.py:16-19.
C6	Naming conventions	✅	ATTR_SAP_QOS_CLASS matches the ATTR_SAP_* family; value constant SCREAMING_SNAKE_CASE.
C7	No unused code	✅	Both constants used in client.py:256 and asserted in both send tests.
C8	No unjustified new dependencies	➖	No dependency changes.
C9	Proto code freshness	➖	No .proto changes.

D: API & Design

#	Criterion	Status	Finding
D1	API future-proofing	✅	No new public API surface; attribute emitted internally in AuditClient.send().
D2	Public API hygiene	✅	New constants live in the internal telemetry constants module; nothing added to a public __all__.
D3	Breaking changes marked	✅	Purely additive — a new attribute on emitted records.
D4	Pagination & tenant filtering	➖	No list/query operation.
D5	Telemetry instrumentation	✅	Change is telemetry; attribute placed alongside existing OTel attributes in the already-instrumented send().
D6	Multi-tenancy support	➖	No change to tenancy behavior.

E: Tests & Documentation

#	Criterion	Status	Finding
E1	Tests added/updated	✅	Resolved. Both paths now assert the QoS attribute: test_send_binary_success (test_client.py:187) and test_send_json_success (test_client.py:205).
E2	Documentation quality	✅	Resolved. user-guide.md:136-137 documents the always-on, non-configurable sap.qos.class="at_least_once" attribute right after the send examples.
E2a	Multi-tenancy documentation	✅	user-guide.md retains its ## Multi-tenancy section (line 335); the edit does not touch it.
E3	Module structure compliance	➖	No new module.

---

❌ Blocking Issues

• None.

⚠️ Non-Blocking Suggestions

• [C1]: Let the re-triggered CI finish; confirm Integration Tests passes (its only prior red was the unrelated agentgateway OAuth2 flake).
• [A1]: Tick the "I have updated documentation" checkbox — the user-guide was updated in this PR.
• [A3]: Add Closes #N if a tracking issue exists.
• Style (nit, pre-existing / out of scope): The sibling literals "sap.tenancy.tenant_id" and "cloudevents.event_id" in the same emit() call remain inline; author has chosen to leave these untouched as they predate this branch. Fine for a separate follow-up.

✅ Things Done Well

• Addressed both prior-review ⚠️ items in a focused follow-up commit: explicit QoS assertion on the JSON path and a user-guide note.
• Key/value extracted to shared constants matching the ATTR_SAP_* convention, with the correct semantic-convention value (at_least_once).
• Constants placed in core/telemetry/constants.py — the right home; auditlog already depends on the telemetry package.
• Minimal, additive, fully-tested diff (4 files, +22/−1).

[jeanscherf]

LGTM