Skip to content

NH-148587 Deprecate service name portion of service key; accept token-only key - #889

Open
tammy-baylis-swi wants to merge 18 commits into
mainfrom
NH-148587-deprecate-service-name-in-key
Open

tammy-baylis-swi wants to merge 18 commits into
mainfrom
NH-148587-deprecate-service-name-in-key

Conversation

@tammy-baylis-swi

@tammy-baylis-swi tammy-baylis-swi commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Token-only service keys as "<api_token>" are now valid.

Requirements for agent-enabled (outside of Lambda) are loosened to "service key is present and at least one character"; previously it had to be formatted as "<api_token>:<service_name>" to be valid.

APM now logs one warning message if service key is provided as "<api_token>:<service_name>", which we consider "legacy" with the next release. Its support will be dropped in a future release. For now, the format is still valid and the <service_name> is used the same way as before, minus the agent-enabled requirement. Log message:

2026-10-15 11:26:43,271 [ solarwinds_apm.apm_config WARNING p#55592.8304157120] The service name component of SW_APM_SERVICE_KEY is deprecated and will be removed in a future release. Set the service name using OTEL_SERVICE_NAME or OTEL_RESOURCE_ATTRIBUTES instead.

If service key is provided as "<api_token>":

  1. service name should have been provided with OTEL_* or by applicable resource detector
    • If none provided, APM is still enabled while OTel SDK sets service name as unknown_service:<executable> or unknown_service
    • If none provided, APM will log one warning to help users with migration towards OTel config (see below)
  2. service key is not re-calculated to include final service name
  3. entire service key/token is masked to only display first 4 and last 4 characters
  4. default OTLP headers are calculated using whole key/token

2026-10-15 12:13:41,248 [ solarwinds_apm.apm_config WARNING p#29328.8304157120] APM is using the service name unknown_service:python because neither OTEL_SERVICE_NAME nor service.name in OTEL_RESOURCE_ATTRIBUTES were set. Configure one to set the service name.

Unit tests and integration tests updated and passing. See ticket for some e2e test results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Lambda naming can overwrite explicit service names, and new tests have logging-isolation and sampler-cleanup issues.

3 open findings
What changed in this PR

Adds token-only service keys to the SolarWinds OpenTelemetry distribution while retaining deprecated legacy keys.

Changes:

  • Relaxes key validation and updates token extraction and masking.
  • Warns about legacy keys and preserves OpenTelemetry service naming for token-only keys.
  • Adds unit and integration coverage.
File Description
tests/​unit/​test_oboe/​test_http_sampler.py Tests token-only sampling and authentication.
tests/​unit/​test_distro.py Tests token extraction and OTLP headers.
tests/​unit/​test_apm_config/​test_apm_config.py Updates configuration and masking tests.
tests/​unit/​test_apm_config/​test_apm_config_service_name.py Tests token-only service naming.
tests/​unit/​test_apm_config/​test_apm_config_service_key_format_deprecated.py Tests legacy-key warnings.
tests/​unit/​test_apm_config/​test_apm_config_agent_enabled.py Tests activation with token-only keys.
tests/​integration/​test_service_name.py Adds token-only naming coverage.
tests/​integration/​test_base_sw_headers_attrs.py Allows test-specific service keys.
solarwinds_apm/​distro.py Accepts token-only keys for authentication headers.
solarwinds_apm/​apm_config.py Updates key handling, warnings, masking, and Lambda name fallback.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread solarwinds_apm/apm_config.py Outdated
Comment thread tests/unit/test_oboe/test_http_sampler.py Outdated
tammy-baylis-swi and others added 5 commits October 15, 2026 10:21
Refactor test setup for span exporter and tracer provider, ensuring proper resource cleanup with a try-finally block.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add logging warning for deprecated legacy service key format.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review identified no blocking issues in the implementation or related coverage; tests were not executed.

0 open findings

3 resolved since last review

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Empty legacy suffixes erase the service-name fallback, and a warning test depends on leaked logging state.

2 open findings

🧠 Review effort: Balanced

Comment thread solarwinds_apm/apm_config.py Outdated
Comment thread tests/unit/test_apm_config/test_apm_config_service_name.py
tammy-baylis-swi and others added 2 commits October 15, 2026 12:21
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Migration warnings incorrectly report missing configuration for explicitly configured service names beginning with unknown_service.

0 open findings

2 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Avoid false warnings for explicitly configured OTel service names

solarwinds_apm/​apm_config.py:454

With a token-only key and OTEL_SERVICE_NAME=unknown_service_api, this prefix check emits a startup warning claiming neither OTel setting was supplied, even though the configured name is retained. Recognize only the SDK fallback forms (unknown_service and unknown_service:<executable>) and account for explicit service-name configuration before warning. Add regression coverage for explicitly configured names sharing this prefix, including names matching a fallback form.

🧠 Review effort: Balanced

@tammy-baylis-swi
tammy-baylis-swi marked this pull request as ready for review October 15, 2026 19:30
@tammy-baylis-swi
tammy-baylis-swi requested review from a team as code owners October 15, 2026 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants