Skip to content

feat(telemetry): warn at startup when required env vars are missing #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeanscherf wants to merge 1 commit into from
Closed

feat(telemetry): warn at startup when required env vars are missing #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "sap-cloud-sdk"
version = "0.35.0"
version = "0.36.0"
description = "SAP Cloud SDK for Python"
readme = "README.md"
license = "Apache-2.0"
Expand Down
23 changes: 21 additions & 2 deletions src/sap_cloud_sdk/core/telemetry/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Configuration for OpenTelemetry telemetry."""

import logging
import os
from dataclasses import dataclass
from typing import Optional
Expand Down Expand Up @@ -44,6 +45,8 @@
ENV_OTLP_PROTOCOL = "OTEL_EXPORTER_OTLP_PROTOCOL"
ENV_OTEL_DISABLED = "CLOUD_SDK_OTEL_DISABLED"

logger = logging.getLogger(__name__)


def _get_region() -> str:
"""Get region from environment or return default."""
Expand Down Expand Up @@ -77,7 +80,16 @@ def _get_system_role() -> str:
System role from APPFND_CONHOS_SYSTEM_ROLE environment variable,
or "ZAFT" if not set.
"""
return os.getenv(ENV_SYSTEM_ROLE, DEFAULT_UNKNOWN)
value = os.getenv(ENV_SYSTEM_ROLE)
if not value:
logger.warning(
"APPFND_CONHOS_SYSTEM_ROLE is not set. "

@gpretto gpretto Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

APPFND_CONHOS_SYSTEM_ROLE still the name? I thought, we suposed to remove the "conhos" from naming.

Should we add something like: "review app.yaml env object" or something like it?

"The sap.cld.system_role telemetry attribute will default to '%s', "
"which may cause malformed telemetry and broken metering.",
DEFAULT_UNKNOWN,
)
return DEFAULT_UNKNOWN
return value


def _get_solution_area() -> str:
Expand Down Expand Up @@ -109,7 +121,14 @@ def _get_ord_id() -> Optional[str]:
Value of ORD_DOCUMENT_ID, or None if the env var is missing or empty.
"""
value = os.getenv(ENV_ORD_DOCUMENT_ID)
return value if value else None
if not value:
logger.warning(
"ORD_DOCUMENT_ID is not set. "
"The sap.ord.id telemetry attribute will be omitted, "
"which may cause incomplete metering telemetry.",
)
return None
return value


def _get_service_display_name() -> Optional[str]:
Expand Down
Loading