Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds feature flag telemetry and tracing support by introducing a FeatureFlagTracing type, updating the correlation context header to include feature flag usage details, and modifying the feature-flag loading logic to populate telemetry metadata and update tracing metrics.
- Introduces
FeatureFlagTracingwith methods to track filters, feature usage, and variant counts - Extends
CreateCorrelationContextHeaderto append feature-flag tracing entries - Updates
loadFeatureFlagsto callpopulateTelemetryMetadataandupdateFeatureFlagTracing, and adds tests for the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| azureappconfiguration/internal/tracing/tracing.go | Added FeatureFlagTracing to header options and included logic to append its data to the correlation context header |
| azureappconfiguration/internal/tracing/featureflag_tracing.go | New implementation of FeatureFlagTracing methods |
| azureappconfiguration/internal/tracing/tracing_test.go | Unit tests for FeatureFlagTracing and header updates |
| azureappconfiguration/constants.go | Added constants for feature-flag keys and tags |
| azureappconfiguration/azureappconfiguration.go | Updated feature-flag loading to populate telemetry metadata, update tracing, and generate reference URLs; added endpoint field |
| azureappconfiguration/azureappconfiguration_test.go | New tests for telemetry-enabled flags and tracing updates |
Comments suppressed due to low confidence (4)
azureappconfiguration/internal/tracing/tracing.go:138
- Use
header.Sethere instead ofheader.Addto ensure the correlation context header is not duplicated if invoked multiple times.
header.Add(CorrelationContextHeader, strings.Join(output, DelimiterComma))
azureappconfiguration/internal/tracing/featureflag_tracing.go:6
- [nitpick] Add GoDoc comments for
FeatureFlagTracingand its exported methods to explain their purpose and usage.
type FeatureFlagTracing struct {
azureappconfiguration/azureappconfiguration.go:809
- [nitpick] Consider adding unit tests for
populateTelemetryMetadatato verify that telemetry metadata (ETag and reference URL) is correctly populated when telemetry is enabled.
func populateTelemetryMetadata(featureFlag map[string]interface{}, setting azappconfig.Setting, endpoint string) {
azureappconfiguration/azureappconfiguration.go:761
- The
fmt.Sprintfcall requires thefmtpackage but I don't see it imported. Please addimport \"fmt\"at the top of the file.
featureFlagReference := fmt.Sprintf("%s/kv/%s", endpoint, *setting.Key)
Member
|
LGTM |
RichardChen820
approved these changes
Jun 24, 2025
linglingye001
added a commit
that referenced
this pull request
Jul 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.