Skip to content

Commit

Permalink
fix: pass parsed versionKey to Tracker (#211)
Browse files Browse the repository at this point in the history 
The AI client wasn't passing the parsed `_ldMeta.versionKey` into newly
created `Tracker`s. The tests didn't catch it because the default value
of strings are `""`, and that's what the tests asserted against.

I've updated the tests to pass in a non-default string to make sure it
is being plumbed through.
  • Loading branch information
@cwaldren-ld
cwaldren-ld authored Dec 5, 2024
1 parent ebe0019 commit 215116a
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 27 deletions.
6 changes: 3 additions & 3 deletions ldai/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ func (c *Client) Config(
// empty object.)
if result.Type() != ldvalue.ObjectType {
c.logConfigWarning(key, "unmarshalling failed, expected JSON object but got %s", result.Type().String())
return defaultValue, newTracker(key, c.sdk, &defaultValue, context, c.logger)
return defaultValue, newTracker(key, "", c.sdk, &defaultValue, context, c.logger)
}

var parsed datamodel.Config
if err := json.Unmarshal([]byte(result.JSONString()), &parsed); err != nil {
c.logConfigWarning(key, "unmarshalling failed: %v", err)
return defaultValue, newTracker(key, c.sdk, &defaultValue, context, c.logger)
return defaultValue, newTracker(key, "", c.sdk, &defaultValue, context, c.logger)
}

mergedVariables := map[string]interface{}{
Expand Down Expand Up @@ -114,7 +114,7 @@ func (c *Client) Config(
}

cfg := builder.Build()
return cfg, newTracker(key, c.sdk, &cfg, context, c.logger)
return cfg, newTracker(key, parsed.Meta.VersionKey, c.sdk, &cfg, context, c.logger)
}

func getAllAttributes(context ldcontext.Context) map[string]interface{} {
Expand Down
8 changes: 4 additions & 4 deletions ldai/tracker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,19 @@ func (d *defaultStopwatch) Stop() time.Duration {
}

// newTracker creates a new Tracker with the specified key, event sink, config, context, and loggers.
func newTracker(key string, events EventSink, config *Config, ctx ldcontext.Context, loggers interfaces.LDLoggers) *Tracker {
return newTrackerWithStopwatch(key, events, config, ctx, loggers, &defaultStopwatch{})
func newTracker(key string, versionKey string, events EventSink, config *Config, ctx ldcontext.Context, loggers interfaces.LDLoggers) *Tracker {
return newTrackerWithStopwatch(key, versionKey, events, config, ctx, loggers, &defaultStopwatch{})
}

// newTrackerWithStopwatch creates a new Tracker with the specified key, event sink, config, context, loggers, and
// stopwatch. This method is used for testing purposes.
func newTrackerWithStopwatch(key string, events EventSink, config *Config, ctx ldcontext.Context, loggers interfaces.LDLoggers, stopwatch Stopwatch) *Tracker {
func newTrackerWithStopwatch(key string, versionKey string, events EventSink, config *Config, ctx ldcontext.Context, loggers interfaces.LDLoggers, stopwatch Stopwatch) *Tracker {
if config == nil {
panic("LaunchDarkly SDK programmer error: config must never be nil")
}

trackData := ldvalue.ObjectBuild().
Set("versionKey", ldvalue.String(config.VersionKey())).
Set("versionKey", ldvalue.String(versionKey)).
Set("configKey", ldvalue.String(key)).Build()

return &Tracker{
Expand Down
40 changes: 20 additions & 20 deletions ldai/tracker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ func (m *mockEvents) TrackMetric(eventName string, context ldcontext.Context, me

func TestTracker_NewPanicsWithNilConfig(t *testing.T) {
assert.Panics(t, func() {
newTracker("key", newMockEvents(), nil, ldcontext.New("key"), nil)
newTracker("key", "versionKey", newMockEvents(), nil, ldcontext.New("key"), nil)
})
}

func TestTracker_NewDoesNotPanicWithConfig(t *testing.T) {
assert.NotPanics(t, func() {
newTracker("key", newMockEvents(), &Config{}, ldcontext.New("key"), nil)
newTracker("key", "versionKey", newMockEvents(), &Config{}, ldcontext.New("key"), nil)
})
}

Expand All @@ -52,22 +52,22 @@ func makeTrackData(configKey, versionKey string) ldvalue.Value {

func TestTracker_TrackSuccess(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)
assert.NoError(t, tracker.TrackSuccess())

expectedEvent := trackEvent{
name: "$ld:ai:generation",
context: ldcontext.New("key"),
metricValue: 1.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

assert.ElementsMatch(t, []trackEvent{expectedEvent}, events.events)
}

func TestTracker_TrackRequest(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)

expectedResponse := ProviderResponse{
Usage: TokenUsage{
Expand All @@ -89,21 +89,21 @@ func TestTracker_TrackRequest(t *testing.T) {
name: "$ld:ai:generation",
context: ldcontext.New("key"),
metricValue: 1,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedDurationEvent := trackEvent{
name: "$ld:ai:duration:total",
context: ldcontext.New("key"),
metricValue: 10.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedTokenUsageEvent := trackEvent{
name: "$ld:ai:tokens:total",
context: ldcontext.New("key"),
metricValue: 1,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedEvents := []trackEvent{expectedSuccessEvent, expectedDurationEvent, expectedTokenUsageEvent}
Expand All @@ -122,7 +122,7 @@ func TestTracker_LatencyMeasuredIfNotProvided(t *testing.T) {
events := newMockEvents()

tracker := newTrackerWithStopwatch(
"key", events, &Config{}, ldcontext.New("key"), nil, mockStopwatch(42*time.Millisecond))
"key", "versionKey", events, &Config{}, ldcontext.New("key"), nil, mockStopwatch(42*time.Millisecond))

expectedResponse := ProviderResponse{
Usage: TokenUsage{
Expand All @@ -145,23 +145,23 @@ func TestTracker_LatencyMeasuredIfNotProvided(t *testing.T) {

func TestTracker_TrackDuration(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)

assert.NoError(t, tracker.TrackDuration(time.Millisecond*10))

expectedEvent := trackEvent{
name: "$ld:ai:duration:total",
context: ldcontext.New("key"),
metricValue: 10.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

assert.ElementsMatch(t, []trackEvent{expectedEvent}, events.events)
}

func TestTracker_TrackFeedback(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)

assert.NoError(t, tracker.TrackFeedback(Positive))
assert.NoError(t, tracker.TrackFeedback(Negative))
Expand All @@ -171,14 +171,14 @@ func TestTracker_TrackFeedback(t *testing.T) {
name: "$ld:ai:feedback:user:positive",
context: ldcontext.New("key"),
metricValue: 1.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedNegativeEvent := trackEvent{
name: "$ld:ai:feedback:user:negative",
context: ldcontext.New("key"),
metricValue: 1.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

assert.ElementsMatch(t, []trackEvent{expectedPositiveEvent, expectedNegativeEvent}, events.events)
Expand All @@ -187,7 +187,7 @@ func TestTracker_TrackFeedback(t *testing.T) {
func TestTracker_TrackUsage(t *testing.T) {
t.Run("only one field set, only one event", func(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)

assert.NoError(t, tracker.TrackUsage(TokenUsage{
Total: 42,
Expand All @@ -197,15 +197,15 @@ func TestTracker_TrackUsage(t *testing.T) {
name: "$ld:ai:tokens:total",
context: ldcontext.New("key"),
metricValue: 42.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

assert.ElementsMatch(t, []trackEvent{expectedEvent}, events.events)
})

t.Run("all fields set, all events", func(t *testing.T) {
events := newMockEvents()
tracker := newTracker("key", events, &Config{}, ldcontext.New("key"), nil)
tracker := newTracker("key", "versionKey", events, &Config{}, ldcontext.New("key"), nil)

assert.NoError(t, tracker.TrackUsage(TokenUsage{
Total: 42,
Expand All @@ -217,21 +217,21 @@ func TestTracker_TrackUsage(t *testing.T) {
name: "$ld:ai:tokens:total",
context: ldcontext.New("key"),
metricValue: 42.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedInput := trackEvent{
name: "$ld:ai:tokens:input",
context: ldcontext.New("key"),
metricValue: 20.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

expectedOutput := trackEvent{
name: "$ld:ai:tokens:output",
context: ldcontext.New("key"),
metricValue: 22.0,
data: makeTrackData("key", ""),
data: makeTrackData("key", "versionKey"),
}

assert.ElementsMatch(t, []trackEvent{expectedTotal, expectedInput, expectedOutput}, events.events)
Expand Down

0 comments on commit 215116a

Please sign in to comment.