Skip to content

Conversation

@RobertPakkoLD
Copy link
Contributor

@RobertPakkoLD RobertPakkoLD commented Aug 5, 2025

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

This is just updating variable names in-line with the new update to the SDK standard:
https://github.com/launchdarkly/sdk-specs/pull/104/files

@kinyoklion kinyoklion changed the title Updating semantic conventions feat: Updating semantic conventions Aug 7, 2025
@kinyoklion kinyoklion marked this pull request as ready for review August 8, 2025 22:51
@kinyoklion kinyoklion requested a review from a team as a code owner August 8, 2025 22:51
@kinyoklion
Copy link
Member

Windows CI failed, but I don't think it is related to this change at all.

@RobertPakkoLD RobertPakkoLD merged commit a73d332 into main Aug 8, 2025
16 of 20 checks passed
@RobertPakkoLD RobertPakkoLD deleted the users/ropakko/updating-semantic-conventions branch August 8, 2025 23:12
kinyoklion pushed a commit that referenced this pull request Aug 12, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](LaunchDarkly.ServerSdk.Telemetry-v1.2.0...LaunchDarkly.ServerSdk.Telemetry-v1.3.0)
(2025-08-11)


### Features

* Support 9x of System.Diagnostics.DiagnosticSource.
([#149](#149))
([d935df6](d935df6))
* Updating semantic conventions
([#148](#148))
([a73d332](a73d332))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
devin-ai-integration bot added a commit to launchdarkly/ruby-server-sdk-otel that referenced this pull request Oct 17, 2025
- Rename feature_flag.context.key to feature_flag.context.id
- Fix feature_flag.provider_name to feature_flag.provider.name (add missing dot)
- Rename feature_flag.variant to feature_flag.result.value
- Add include_value configuration option (keep include_variant as deprecated)
- Add feature_flag.result.reason.inExperiment attribute
- Add feature_flag.result.variationIndex attribute
- Add feature_flag.set.id attribute with environment_id configuration support
- Update all tests to use new attribute names
- Add tests for new attributes

Follows OpenTelemetry semantic conventions specification:
https://raw.githubusercontent.com/launchdarkly/sdk-specs/refs/heads/main/specs/OTEL-openteletry-integration/README.md

Reference implementations:
- Go SDK: launchdarkly/go-server-sdk#292
- .NET SDK: launchdarkly/dotnet-core#148

Co-Authored-By: Vadim Korolik <[email protected]>
Vadman97 added a commit to launchdarkly/ruby-server-sdk-otel that referenced this pull request Oct 17, 2025
## Summary

Updates the Ruby OpenTelemetry tracing hook to comply with the latest
OpenTelemetry semantic conventions specification for feature flag
instrumentation. This brings the Ruby SDK in line with other
LaunchDarkly SDKs (Java, Go, .NET).

**Link to Devin run**:
https://app.devin.ai/sessions/00115de67e494cc999f5c247414cd9b1
**Requested by**: @Vadman97

## Changes

### Attribute Name Updates (Breaking for Observability Consumers)
- `feature_flag.context.key` → `feature_flag.context.id`
- `feature_flag.provider_name` → `feature_flag.provider.name`  
- `feature_flag.variant` → `feature_flag.result.value`

### New Configuration Options
- **`include_value`**: Replaces deprecated `include_variant` option.
Controls whether flag values are included in telemetry.
- **`environment_id`**: Optional string parameter that adds
`feature_flag.set.id` attribute. Validates non-empty strings and logs
warnings for invalid input.

### New Optional Attributes
- **`feature_flag.result.reason.inExperiment`**: Set to `true` when
evaluation is part of an experiment
- **`feature_flag.result.variationIndex`**: Includes variation index
when available

### Backward Compatibility
- `include_variant` option continues to work via fallback logic:
`opts.fetch(:include_value, opts.fetch(:include_variant, false))`
- Added test coverage for deprecated option to ensure backward
compatibility

### Bug Fixes
- Fixed initialization order bug where `validate_environment_id` was
called before `@logger` was initialized, causing `NoMethodError` on Ruby
3.0/3.1

## Testing

- All existing tests updated to use new attribute names
- New test contexts added for:
  - `include_value` configuration
  - `include_variant` backward compatibility
  - `environment_id` configuration (valid and invalid cases)
  - `inExperiment` and `variationIndex` attributes
- All CI checks passing on Ruby 3.0, 3.1, 3.2, jruby-9.4, and Windows

## Review Checklist

**Requirements**

- [x] I have added test coverage for new or changed functionality
- [x] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions

**⚠️ Critical Review Items**

1. **Test logic for `inExperiment`** (line 221 in spec): The test
"includes inExperiment when evaluation is part of experiment" verifies
the attribute is NOT present (`.to be false`). Please verify this is
correct behavior - is the test setup actually creating an experiment
scenario, or is this testing the default case?

2. **Backward compatibility**: Verify the nested fallback pattern
`opts.fetch(:include_value, opts.fetch(:include_variant, false))` works
correctly for all combinations of options.

3. **Initialization order**: The constructor now sets `@logger` before
calling `validate_environment_id`. Verify this doesn't break any
assumptions about initialization sequence.

4. **Environment validation**: The `environment_id` validation only
checks for non-empty strings. Confirm this matches the spec requirements
(no format/character restrictions needed?).

## Related Issues

- Specification:
https://raw.githubusercontent.com/launchdarkly/sdk-specs/refs/heads/main/specs/OTEL-openteletry-integration/README.md
- Reference implementations:
  - Java SDK: launchdarkly/java-core#89
  - .NET SDK: launchdarkly/dotnet-core#148
  - Go SDK: launchdarkly/go-server-sdk#292

## Additional Context

- Unable to run Rubocop locally due to missing Ruby development headers
in environment. All lint verification was done via CI.
- Changes are focused on the `ruby-server-sdk-otel` package; the main
`ruby-server-sdk` package does not require updates.
Vadman97 pushed a commit to launchdarkly/java-core that referenced this pull request Oct 23, 2025
## Summary

Updates the Java OpenTelemetry tracing hook to align with the [latest
semantic conventions for feature flag
evaluation](https://github.com/launchdarkly/sdk-specs/blob/main/specs/OTEL-openteletry-integration/README.md).
This includes renaming existing attributes, adding new optional
attributes, and updating the configuration API while maintaining
backward compatibility.

Reference implementations:
[dotnet-core#148](launchdarkly/dotnet-core#148),
[go-server-sdk#292](launchdarkly/go-server-sdk#292)

## Changes

### Attribute Name Updates (Breaking Change)
- `feature_flag.provider_name` → `feature_flag.provider.name`
- `feature_flag.variant` → `feature_flag.result.value`
- `feature_flag.context.key` → `feature_flag.context.id`

### New Optional Attributes
- `feature_flag.result.variationIndex` - Added when variation index is
not `NO_VARIATION` (-1)
- `feature_flag.result.reason.inExperiment` - Added when
`isInExperiment()` returns true

### API Changes
- Added `Builder.withValue()` method (recommended)
- Deprecated `Builder.withVariant()` method (still functional,
internally calls `withValue()`)
- Renamed internal field from `withVariant` to `withValue`

### Implementation Details
- Both `beforeEvaluationInternal` and `afterEvaluation` now include the
provider name attribute
- New attributes are conditionally added based on evaluation details
- Test assertions updated to account for the additional `variationIndex`
attribute

## Review Checklist

**Critical items for review:**
- [ ] Verify attribute names exactly match the [OpenTelemetry semantic
conventions
spec](https://github.com/launchdarkly/sdk-specs/blob/main/specs/OTEL-openteletry-integration/README.md)
- [ ] Confirm conditional logic for `isInExperiment()` and
`variationIndex` is correct
- [ ] Review backward compatibility strategy for deprecated
`withVariant()` method
- [ ] Verify test coverage adequately validates the new optional
attributes

**Notes:**
- The `feature_flag.set.id` attribute is not included because the Java
SDK's `EvaluationSeriesContext` does not currently have an
`environmentId` field (consistent with the Go implementation)
- CI failures appear to be unrelated dependency resolution issues
(`nanohttpd` not found) in other modules
- Local tests for the `java-server-sdk-otel` module pass successfully

---

**Requirements**

- [x] I have added test coverage for new or changed functionality
- [x] I have followed the repository's [pull request submission
guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests)
- [x] I have validated my changes against all supported platform
versions

**Related issues**

Request from [email protected] to update Java OpenTelemetry tracing
hook to latest semantic conventions.

**Additional context**

- Link to Devin run:
https://app.devin.ai/sessions/e4f4cdd8b60047ad9402001186442975
- Requested by: [email protected]

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Ryan Lamb <[email protected]>
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