Skip to content

feat: adds service instance id to OTEL attributes - #5849

Merged
akshaydeo merged 1 commit into
devfrom
08-05-feat_adds_service_instance_id_to_otel_attributes
Aug 4, 2026
Merged

akshaydeo merged 1 commit into
devfrom
08-05-feat_adds_service_instance_id_to_otel_attributes

Conversation

@roroghost17

@roroghost17 roroghost17 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

Changes

  • NormalizeModelName fine-tune short-circuit: OpenAI fine-tune IDs prefixed with ft: are now returned immediately before BaseModelName is called, preventing a date-like custom suffix (e.g. custom-20250514) from being stripped as if it were a Bedrock version tag.
  • getStringSliceAttr index preservation: When a []any attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that entitySetFromAttrs can correctly pair and filter them.
  • OTel span filtering: Spans with both an empty provider and an empty model are skipped before final-span selection in recordMetricsFromTrace, preventing empty-label metric series from being emitted.
  • serviceInstanceID as a package-level variable: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a service_instance_id datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
  • Prometheus ActiveRequests decrement on pre-dispatch rejection: PostLLMHook now decrements ActiveRequests before returning early for no-provider/no-model requests, fixing a counter leak introduced when PreLLMHook incremented it.
  • Key rotation event uses normalized model name: KeyRotationEventsTotal now records the normalized model value instead of originalModel, keeping label values consistent with other metrics.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
  • TestNormalizeModelName covers the new ft:gpt-4o-mini:acme:custom-20250514 case.
  • TestGetStringSliceAttr_AnyPreservesIndex verifies that non-string elements produce an empty string at the correct index.
  • TestEntitySetFromAttrs_MixedAnyKeepsAlignment verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

Breaking changes

  • Yes
  • No

Related issues

Security considerations

None.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a7144b6-ccce-48d2-b761-c07b2255b9fb

📥 Commits

Reviewing files that changed from the base of the PR and between 638ffe7 and 9c30fef.

📒 Files selected for processing (6)
  • core/schemas/normalizemodelname_test.go
  • core/schemas/utils.go
  • plugins/otel/entityset_test.go
  • plugins/otel/main.go
  • plugins/otel/metrics.go
  • plugins/telemetry/main.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • plugins/otel/metrics.go
  • core/schemas/normalizemodelname_test.go
  • plugins/otel/entityset_test.go
  • plugins/telemetry/main.go
  • plugins/otel/main.go
  • core/schemas/utils.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Preserved OpenAI fine-tune model identifiers with date-like custom suffixes.
    • Improved OpenTelemetry attribute alignment and entity pairing.
    • Excluded incomplete LLM and retry spans from per-attempt metrics.
    • Corrected active-request gauge cleanup for rejected requests.
    • Standardized model names in key-rotation metrics.
  • Improvements

    • Added consistent service instance identifiers to telemetry resources and metrics, with hostname fallback support.
    • Improved telemetry attribute documentation for entity sets.

Walkthrough

NormalizeModelName preserves fine-tune identifiers. OpenTelemetry now preserves entity alignment, shares a service instance ID, and filters unlabeled spans. Telemetry metrics correct gauge cleanup and key-rotation model labels.

Changes

Normalization and telemetry updates

Layer / File(s) Summary
Fine-tune model-name preservation
core/schemas/utils.go, core/schemas/normalizemodelname_test.go
NormalizeModelName preserves complete ft: identifiers, including date-like suffixes. Regression coverage was added.
OpenTelemetry service identity and attributes
plugins/otel/metrics.go, plugins/otel/main.go, plugins/otel/entityset_test.go
OpenTelemetry metrics use a shared service instance ID. String-slice conversion preserves positions. Entity alignment tests were added.
OpenTelemetry trace metric filtering
plugins/otel/main.go
Trace extraction skips LLM and retry spans without provider and model labels.
Telemetry metric label corrections
plugins/telemetry/main.go
Rejected pre-dispatch requests decrement the active-request gauge. Key-rotation metrics use normalized model names.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • maximhq/bifrost#5847: This PR modifies and tests the same model-normalization and OTEL/telemetry metric-handling paths.

Suggested reviewers: akshaydeo, sammaji

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the addition of the service instance ID to OpenTelemetry attributes, which is a primary change in the pull request.
Description check ✅ Passed The description covers the required sections, explains the changes, identifies affected areas, and provides targeted test commands and cases.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 08-05-feat_adds_service_instance_id_to_otel_attributes

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

roroghost17 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@CLAassistant

CLAassistant commented Aug 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@roroghost17
roroghost17 marked this pull request as ready for review August 4, 2026 20:09
@coderabbitai
coderabbitai Bot requested review from akshaydeo and sammaji August 4, 2026 20:10
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 4, 2026

akshaydeo commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Aug 4, 10:02 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 4, 10:08 PM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 10:09 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 08-05-feat_add_team___customer___bu_ids_and_names_to_otel_metrics to graphite-base/5849 August 4, 2026 22:04
@akshaydeo
akshaydeo changed the base branch from graphite-base/5849 to dev August 4, 2026 22:06
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review August 4, 2026 22:06

The base branch was changed.

@akshaydeo
akshaydeo force-pushed the 08-05-feat_adds_service_instance_id_to_otel_attributes branch from 6ae9015 to 9c30fef Compare August 4, 2026 22:07
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@akshaydeo
akshaydeo merged commit adfb312 into dev Aug 4, 2026
14 of 15 checks passed
@akshaydeo
akshaydeo deleted the 08-05-feat_adds_service_instance_id_to_otel_attributes branch August 4, 2026 22:09
akshaydeo pushed a commit that referenced this pull request Aug 5, 2026
## Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

## Changes

- **`NormalizeModelName` fine-tune short-circuit**: OpenAI fine-tune IDs prefixed with `ft:` are now returned immediately before `BaseModelName` is called, preventing a date-like custom suffix (e.g. `custom-20250514`) from being stripped as if it were a Bedrock version tag.
- **`getStringSliceAttr` index preservation**: When a `[]any` attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that `entitySetFromAttrs` can correctly pair and filter them.
- **OTel span filtering**: Spans with both an empty provider and an empty model are skipped before final-span selection in `recordMetricsFromTrace`, preventing empty-label metric series from being emitted.
- **`serviceInstanceID` as a package-level variable**: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a `service_instance_id` datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
- **Prometheus `ActiveRequests` decrement on pre-dispatch rejection**: `PostLLMHook` now decrements `ActiveRequests` before returning early for no-provider/no-model requests, fixing a counter leak introduced when `PreLLMHook` incremented it.
- **Key rotation event uses normalized model name**: `KeyRotationEventsTotal` now records the normalized `model` value instead of `originalModel`, keeping label values consistent with other metrics.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
```

- `TestNormalizeModelName` covers the new `ft:gpt-4o-mini:acme:custom-20250514` case.
- `TestGetStringSliceAttr_AnyPreservesIndex` verifies that non-string elements produce an empty string at the correct index.
- `TestEntitySetFromAttrs_MixedAnyKeepsAlignment` verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akshaydeo pushed a commit that referenced this pull request Aug 7, 2026
## Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

## Changes

- **`NormalizeModelName` fine-tune short-circuit**: OpenAI fine-tune IDs prefixed with `ft:` are now returned immediately before `BaseModelName` is called, preventing a date-like custom suffix (e.g. `custom-20250514`) from being stripped as if it were a Bedrock version tag.
- **`getStringSliceAttr` index preservation**: When a `[]any` attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that `entitySetFromAttrs` can correctly pair and filter them.
- **OTel span filtering**: Spans with both an empty provider and an empty model are skipped before final-span selection in `recordMetricsFromTrace`, preventing empty-label metric series from being emitted.
- **`serviceInstanceID` as a package-level variable**: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a `service_instance_id` datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
- **Prometheus `ActiveRequests` decrement on pre-dispatch rejection**: `PostLLMHook` now decrements `ActiveRequests` before returning early for no-provider/no-model requests, fixing a counter leak introduced when `PreLLMHook` incremented it.
- **Key rotation event uses normalized model name**: `KeyRotationEventsTotal` now records the normalized `model` value instead of `originalModel`, keeping label values consistent with other metrics.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
```

- `TestNormalizeModelName` covers the new `ft:gpt-4o-mini:acme:custom-20250514` case.
- `TestGetStringSliceAttr_AnyPreservesIndex` verifies that non-string elements produce an empty string at the correct index.
- `TestEntitySetFromAttrs_MixedAnyKeepsAlignment` verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@akshaydeo akshaydeo mentioned this pull request Aug 7, 2026
18 tasks
atharvamhaske pushed a commit to atharvamhaske/bifrost that referenced this pull request Aug 13, 2026
## Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

## Changes

- **`NormalizeModelName` fine-tune short-circuit**: OpenAI fine-tune IDs prefixed with `ft:` are now returned immediately before `BaseModelName` is called, preventing a date-like custom suffix (e.g. `custom-20250514`) from being stripped as if it were a Bedrock version tag.
- **`getStringSliceAttr` index preservation**: When a `[]any` attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that `entitySetFromAttrs` can correctly pair and filter them.
- **OTel span filtering**: Spans with both an empty provider and an empty model are skipped before final-span selection in `recordMetricsFromTrace`, preventing empty-label metric series from being emitted.
- **`serviceInstanceID` as a package-level variable**: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a `service_instance_id` datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
- **Prometheus `ActiveRequests` decrement on pre-dispatch rejection**: `PostLLMHook` now decrements `ActiveRequests` before returning early for no-provider/no-model requests, fixing a counter leak introduced when `PreLLMHook` incremented it.
- **Key rotation event uses normalized model name**: `KeyRotationEventsTotal` now records the normalized `model` value instead of `originalModel`, keeping label values consistent with other metrics.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
```

- `TestNormalizeModelName` covers the new `ft:gpt-4o-mini:acme:custom-20250514` case.
- `TestGetStringSliceAttr_AnyPreservesIndex` verifies that non-string elements produce an empty string at the correct index.
- `TestEntitySetFromAttrs_MixedAnyKeepsAlignment` verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

## Changes

- **`NormalizeModelName` fine-tune short-circuit**: OpenAI fine-tune IDs prefixed with `ft:` are now returned immediately before `BaseModelName` is called, preventing a date-like custom suffix (e.g. `custom-20250514`) from being stripped as if it were a Bedrock version tag.
- **`getStringSliceAttr` index preservation**: When a `[]any` attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that `entitySetFromAttrs` can correctly pair and filter them.
- **OTel span filtering**: Spans with both an empty provider and an empty model are skipped before final-span selection in `recordMetricsFromTrace`, preventing empty-label metric series from being emitted.
- **`serviceInstanceID` as a package-level variable**: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a `service_instance_id` datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
- **Prometheus `ActiveRequests` decrement on pre-dispatch rejection**: `PostLLMHook` now decrements `ActiveRequests` before returning early for no-provider/no-model requests, fixing a counter leak introduced when `PreLLMHook` incremented it.
- **Key rotation event uses normalized model name**: `KeyRotationEventsTotal` now records the normalized `model` value instead of `originalModel`, keeping label values consistent with other metrics.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
```

- `TestNormalizeModelName` covers the new `ft:gpt-4o-mini:acme:custom-20250514` case.
- `TestGetStringSliceAttr_AnyPreservesIndex` verifies that non-string elements produce an empty string at the correct index.
- `TestEntitySetFromAttrs_MixedAnyKeepsAlignment` verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
## Summary

Several correctness fixes across model name normalization, OTel metrics alignment, and Prometheus active-request tracking. The common theme is preventing data loss or label pollution caused by edge-case inputs (fine-tune model IDs with date-like suffixes, mixed-type attribute arrays, spans with no provider/model, and pre-dispatch rejections that left active-request counters incremented).

## Changes

- **`NormalizeModelName` fine-tune short-circuit**: OpenAI fine-tune IDs prefixed with `ft:` are now returned immediately before `BaseModelName` is called, preventing a date-like custom suffix (e.g. `custom-20250514`) from being stripped as if it were a Bedrock version tag.
- **`getStringSliceAttr` index preservation**: When a `[]any` attribute value contains non-string elements, the slot is now kept as an empty string rather than being dropped. This keeps id and name arrays index-aligned so that `entitySetFromAttrs` can correctly pair and filter them.
- **OTel span filtering**: Spans with both an empty provider and an empty model are skipped before final-span selection in `recordMetricsFromTrace`, preventing empty-label metric series from being emitted.
- **`serviceInstanceID` as a package-level variable**: The hostname/fallback resolution is now computed once at startup and reused in both the resource attribute and as a `service_instance_id` datapoint label, so per-replica breakdown survives collector configurations that drop resource attributes.
- **Prometheus `ActiveRequests` decrement on pre-dispatch rejection**: `PostLLMHook` now decrements `ActiveRequests` before returning early for no-provider/no-model requests, fixing a counter leak introduced when `PreLLMHook` incremented it.
- **Key rotation event uses normalized model name**: `KeyRotationEventsTotal` now records the normalized `model` value instead of `originalModel`, keeping label values consistent with other metrics.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/... ./plugins/otel/... ./plugins/telemetry/...
```

- `TestNormalizeModelName` covers the new `ft:gpt-4o-mini:acme:custom-20250514` case.
- `TestGetStringSliceAttr_AnyPreservesIndex` verifies that non-string elements produce an empty string at the correct index.
- `TestEntitySetFromAttrs_MixedAnyKeepsAlignment` verifies that a non-string ID element drops both that ID and its paired name without shifting remaining entries.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
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.

3 participants