Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Oct 17, 2025

Summary

Updates the Java OpenTelemetry tracing hook to align with the latest semantic conventions for feature flag evaluation. This includes renaming existing attributes, adding new optional attributes, and updating the configuration API while maintaining backward compatibility.

Reference implementations: dotnet-core#148, go-server-sdk#292

Changes

Attribute Name Updates (Breaking Change)

  • feature_flag.provider_namefeature_flag.provider.name
  • feature_flag.variantfeature_flag.result.value
  • feature_flag.context.keyfeature_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
  • 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

  • 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

Related issues

Request from rlamb@launchdarkly.com to update Java OpenTelemetry tracing hook to latest semantic conventions.

Additional context

- Update attribute names to match latest semantic conventions:
  - feature_flag.provider_name -> feature_flag.provider.name
  - feature_flag.variant -> feature_flag.result.value
  - feature_flag.context.key -> feature_flag.context.id
- Add new optional attributes:
  - feature_flag.result.reason.inExperiment
  - feature_flag.result.variationIndex
- Deprecate withVariant() method, add withValue() for backward compatibility
- Update tests to use new constant names and expect new attributes

Follows specification: https://github.com/launchdarkly/sdk-specs/blob/main/specs/OTEL-openteletry-integration/README.md
Reference implementations: dotnet-core#148, go-server-sdk#292

Co-Authored-By: rlamb@launchdarkly.com <rlamb@launchdarkly.com>
@devin-ai-integration devin-ai-integration bot requested a review from a team as a code owner October 17, 2025 15:47
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@Vadman97 Vadman97 requested a review from a team October 17, 2025 15:48
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.
Restored SEMCONV_FEATURE_FLAG_PROVIDER_NAME attribute in beforeEvaluationInternal
that was accidentally removed. The provider name should be included in both
the span attributes (beforeEvaluationInternal) and event attributes (afterEvaluation).

Addresses PR feedback from tanderson-ld and kinyoklion.

Co-Authored-By: rlamb@launchdarkly.com <rlamb@launchdarkly.com>
@kinyoklion kinyoklion added the devin-pr Label for PR's created using devin. label Oct 18, 2025
@Vadman97 Vadman97 merged commit c67ce73 into main Oct 23, 2025
20 of 21 checks passed
@Vadman97 Vadman97 deleted the devin/1760715862-update-otel-semantic-conventions branch October 23, 2025 00:10
kinyoklion pushed a commit that referenced this pull request Oct 23, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.2.0](lib/java-server-sdk-otel-0.1.0...lib/java-server-sdk-otel-0.2.0)
(2025-10-23)


### Features

* Update OpenTelemetry semantic conventions
([#89](#89))
([c67ce73](c67ce73))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devin-pr Label for PR's created using devin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants