Skip to content

fix(bedrock): keep dotted model names in cross-region profile IDs (us.openai.gpt-5.6-sol) - #4503

Open
kimnamu wants to merge 1 commit into
traceloop:mainfrom
kimnamu:fix/bedrock-gpt-6-dotted-model-id
Open

kimnamu wants to merge 1 commit into
traceloop:mainfrom
kimnamu:fix/bedrock-gpt-6-dotted-model-id

Conversation

@kimnamu

@kimnamu kimnamu commented Sep 24, 2026 •

Copy link
Copy Markdown

Thanks for maintaining the Bedrock instrumentation and for the ARN / cross-region parsing added in #2785, which this touches. The prefix-less branch of _cross_region_check already splits with split(".", 1); this limits the geo-prefixed branch the same way, so a dot inside the model name survives.

Fixes #4501

Change (__init__.py, 1 line): parts = value.split(".") becomes value.split(".", 2), which yields exactly prefix, vendor and model.

Before / After (real converse calls in us-east-1; the ARN, no-prefix and eu. rows come from the unit tests)

Model ID Before (main d13e721) After
us.openai.gpt-5.6-sol ❌ gpt-5, span chat gpt-5 ✅ gpt-5.6-sol, span chat gpt-5.6-sol
us.openai.gpt-5.6-luna ❌ gpt-5, span chat gpt-5 ✅ gpt-5.6-luna, span chat gpt-5.6-luna
us.xai.grok-4.6 ❌ grok-4, span chat grok-4 ✅ grok-4.6, span chat grok-4.6
arn:...:inference-profile/us.openai.gpt-5.6-sol ❌ gpt-5 ✅ gpt-5.6-sol
openai.gpt-5.6-sol (no prefix) ✅ gpt-5.6-sol ✅ unchanged
us.openai.gpt-6-sol / -luna / -astra ✅ gpt-6-sol / gpt-6-luna / gpt-6-astra ✅ unchanged
eu.anthropic.claude-3-7-sonnet-..., other IDs ✅ ✅ unchanged, 377 existing tests pass

This is two lines below the prefix list my open #4235 extends with global. On its own #4235 would record global.openai.gpt-5.6-sol as gpt-5 too. The branches merge without conflict; merged, the suite passes (386) and global.openai.gpt-5.6-sol resolves to gpt-5.6-sol.

Tests: new tests/test_model_id_parsing.py, 4 cases (profile ID, profile ARN, prefix-less ID, undotted IDs).

RED / GREEN, full suite, lint

Source reverted, new tests kept:

E       AssertionError: assert ('aws.bedrock...nai', 'gpt-5') == ('aws.bedrock...'gpt-5.6-sol')
E         At index 2 diff: 'gpt-5' != 'gpt-5.6-sol'
FAILED tests/test_model_id_parsing.py::TestDottedModelName::test_regional_profile_keeps_full_model_name
FAILED tests/test_model_id_parsing.py::TestDottedModelName::test_regional_profile_arn_keeps_full_model_name
2 failed, 2 passed, 1 warning in 0.08s

Restored:

$ uv run pytest tests/test_model_id_parsing.py
4 passed, 1 warning in 0.01s
$ uv run pytest tests/ --record-mode=none
381 passed, 139 warnings in 6.34s
$ uv run ruff check .
All checks passed!
  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Summary by CodeRabbit

  • Bug Fixes
    • Improved parsing of Bedrock model identifiers when model names contain dots, including cross-region and regional inference-profile identifiers. Identifiers without dotted model names continue to be handled as before.

….openai.gpt-5.6-sol)

_cross_region_check split geo-prefixed IDs on every dot and kept only
the piece after the vendor, so us.openai.gpt-5.6-sol (and its
inference-profile ARN) resolved to model "gpt-5" in the span name and
gen_ai.request.model. The prefix-less branch already uses split(".", 1);
limit the prefixed split to the prefix and vendor the same way.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 4e6b71ac-4c38-4d5f-8c60-bc4c181fe324

📥 Commits

Reviewing files that changed from the base of the PR and between d13e721 and 743508e.

📒 Files selected for processing (2)
  • packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py
  • packages/opentelemetry-instrumentation-bedrock/tests/test_model_id_parsing.py

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Bedrock cross-region model ID parser now preserves dots in model names. New tests cover regional IDs, inference-profile ARNs, prefix-less IDs, and undotted model names.

Changes

Bedrock model ID parsing

Layer / File(s) Summary
Preserve dotted model names
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/__init__.py, packages/opentelemetry-instrumentation-bedrock/tests/test_model_id_parsing.py
_cross_region_check limits splitting to the first two dots, retaining the remaining model name. Tests check dotted names in regional IDs and ARNs, prefix-less IDs, and undotted regional IDs.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 74350

The change addresses truncated dotted model names while preserving the inspected existing ID forms. No actionable merge risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving dotted model names in Bedrock cross-region profile IDs.
Linked Issues check ✅ Passed The change satisfies issue #4501. _cross_region_check now uses value.split(".", 2), so geo-prefixed IDs preserve dots in the model name. The added tests cover us.openai.gpt-5.6-sol, its inferenc…
Out of Scope Changes check ✅ Passed The reported changes are limited to the Bedrock model ID parsing fix and focused tests for issue #4501. No unrelated behavior or files are identified in the pull request summary.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: Bedrock profile IDs with a dot in the model name (us.openai.gpt-5.6-sol) are recorded as model 'gpt-5'

1 participant