Skip to content

Commit 125eb04

Browse files
committed
add query in test
Signed-off-by: Harshil Gupta <[email protected]>
1 parent d8379d7 commit 125eb04

File tree

4 files changed

+117
-22
lines changed

4 files changed

+117
-22
lines changed

internal/storage/metricstore/elasticsearch/query_builder.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ func (*QueryBuilder) buildTimeSeriesAggQuery(params metricstore.BaseQueryParamet
115115
return dateHistAgg
116116
}
117117

118+
// This function builds an Elasticsearch query to fetch unique values for a specified attribute (tag), optionally filtered by a service name.
118119
func (*QueryBuilder) BuildAttributeValuesQuery(params *metricstore.AttributeValuesQueryParameters) (elastic.Query, elastic.Aggregation) {
119120
// Create a bool query to filter by service name if provided
120121
boolQuery := elastic.NewBoolQuery()
@@ -129,14 +130,13 @@ func (*QueryBuilder) BuildAttributeValuesQuery(params *metricstore.AttributeValu
129130
// Create nested aggregation for each path
130131
nestedAgg := elastic.NewNestedAggregation().Path(path)
131132

132-
// Filter by the specified key
133133
filterAgg := elastic.NewFilterAggregation().
134134
Filter(elastic.NewTermQuery(path+".key", params.AttributeKey))
135135

136136
// Get unique values
137137
valuesAgg := elastic.NewTermsAggregation().
138138
Field(path + ".value").
139-
Size(10000)
139+
Size(100)
140140

141141
// Chain aggregations
142142
filterAgg.SubAggregation("values", valuesAgg)

internal/storage/metricstore/elasticsearch/reader.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,11 @@ func (r MetricsReader) executeSearchWithAggregation(
223223
query elastic.Query,
224224
aggQuery elastic.Aggregation,
225225
) (*elastic.SearchResult, error) {
226-
// Calculate a default time range for the last day
226+
// Calculate a default time range for the last hour, keeping this low to reduce data volume
227227
timeRange := TimeRange{
228-
startTimeMillis: time.Now().Add(-24 * time.Hour).UnixMilli(),
228+
startTimeMillis: time.Now().Add(-1 * time.Hour).UnixMilli(),
229229
endTimeMillis: time.Now().UnixMilli(),
230-
extendedStartTimeMillis: time.Now().Add(-24 * time.Hour).UnixMilli(),
230+
extendedStartTimeMillis: time.Now().Add(-1 * time.Hour).UnixMilli(),
231231
}
232232

233233
// Here we'll execute using a method similar to the QueryBuilder's Execute

internal/storage/metricstore/elasticsearch/reader_test.go

Lines changed: 111 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,94 @@ var mockErrorRateQuery = `{
131131
"buckets_path": "_count"}}}}}}
132132
`
133133

134+
var mockAttributeValuesQuery = `{
135+
"size": 0,
136+
"query": {
137+
"bool": {
138+
"filter": [
139+
{
140+
"term": {
141+
"process.serviceName": "service1"
142+
}
143+
}
144+
]
145+
}
146+
},
147+
"aggs": {
148+
"results_buckets": {
149+
"global": {},
150+
"aggs": {
151+
"path_0": {
152+
"nested": {
153+
"path": "tag"
154+
},
155+
"aggs": {
156+
"filtered_by_key": {
157+
"filter": {
158+
"term": {
159+
"tag.key": "environment"
160+
}
161+
},
162+
"aggs": {
163+
"values": {
164+
"terms": {
165+
"field": "tag.value",
166+
"size": 100
167+
}
168+
}
169+
}
170+
}
171+
}
172+
},
173+
"path_1": {
174+
"nested": {
175+
"path": "process.tag"
176+
},
177+
"aggs": {
178+
"filtered_by_key": {
179+
"filter": {
180+
"term": {
181+
"process.tag.key": "environment"
182+
}
183+
},
184+
"aggs": {
185+
"values": {
186+
"terms": {
187+
"field": "process.tag.value",
188+
"size": 100
189+
}
190+
}
191+
}
192+
}
193+
}
194+
},
195+
"path_2": {
196+
"nested": {
197+
"path": "logs.fields"
198+
},
199+
"aggs": {
200+
"filtered_by_key": {
201+
"filter": {
202+
"term": {
203+
"logs.fields.key": "environment"
204+
}
205+
},
206+
"aggs": {
207+
"values": {
208+
"terms": {
209+
"field": "logs.fields.value",
210+
"size": 100
211+
}
212+
}
213+
}
214+
}
215+
}
216+
}
217+
}
218+
}
219+
}
220+
}`
221+
134222
const (
135223
mockEsValidResponse = "testdata/output_valid_es.json"
136224
mockCallRateResponse = "testdata/output_call_rate.json"
@@ -921,16 +1009,18 @@ func TestGetLabelValues(t *testing.T) {
9211009
responseFile string
9221010
params *metricstore.AttributeValuesQueryParameters
9231011
expectedValues []string
1012+
expectedQuery string
9241013
expectError bool
9251014
}{
9261015
{
9271016
name: "successful lookup for string values",
9281017
responseFile: "testdata/output_attribute_key_values.json",
9291018
params: &metricstore.AttributeValuesQueryParameters{
930-
AttributeKey: "span_kind",
1019+
AttributeKey: "environment",
9311020
ServiceName: "service1",
9321021
},
9331022
expectedValues: []string{"server", "client", "producer"},
1023+
expectedQuery: mockAttributeValuesQuery,
9341024
expectError: false,
9351025
},
9361026
{
@@ -947,21 +1037,8 @@ func TestGetLabelValues(t *testing.T) {
9471037

9481038
for _, tc := range testCases {
9491039
t.Run(tc.name, func(t *testing.T) {
950-
// Define a custom version of the mockServer to help debug
951-
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
952-
w.Header().Set("Content-Type", "application/json")
953-
w.WriteHeader(http.StatusOK)
954-
955-
// Handle initial ping request
956-
if r.Method == http.MethodHead || r.URL.Path == "/" {
957-
sendResponse(t, w, "testdata/output_valid_es.json")
958-
return
959-
}
1040+
mockServer := startMockEsServer(t, tc.expectedQuery, tc.responseFile)
9601041

961-
// For actual queries, send our test response
962-
t.Logf("Received request for %s, returning file: %s", r.URL.Path, tc.responseFile)
963-
sendResponse(t, w, tc.responseFile)
964-
}))
9651042
defer mockServer.Close()
9661043

9671044
// Get the reader with our mock server
@@ -1139,7 +1216,25 @@ func compareBoolQuery(t *testing.T, expected, actual map[string]any) {
11391216

11401217
// Compare filters (excluding time ranges)
11411218
if expectedFilters, ok := expectedBool["filter"].([]any); ok {
1142-
actualFilters := actualBool["filter"].([]any)
1219+
// Handle the case where actual filter is either array or single object
1220+
actualFilter, exists := actualBool["filter"]
1221+
if !exists {
1222+
t.Errorf("Expected filter but not found in actual query")
1223+
return
1224+
}
1225+
1226+
// Convert to array if it's not already
1227+
var actualFilters []any
1228+
switch f := actualFilter.(type) {
1229+
case []any:
1230+
actualFilters = f
1231+
case map[string]any:
1232+
actualFilters = []any{f}
1233+
default:
1234+
t.Errorf("Filter has unexpected type: %T", f)
1235+
return
1236+
}
1237+
11431238
compareFilters(t, expectedFilters, actualFilters)
11441239
}
11451240
}

internal/storage/metricstore/prometheus/metricstore/reader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ func (m MetricsReader) GetAttributeValues(ctx context.Context, params *metricsto
270270
// Build match[] selectors to filter by service names if provided
271271
var matchers []string
272272
if params.ServiceName != "" {
273-
matchers = append(matchers, fmt.Sprintf("{service_name=\"%s\"}", params.ServiceName))
273+
matchers = append(matchers, fmt.Sprintf("{service_name=%q}", params.ServiceName))
274274
}
275275

276276
values, warnings, err := m.client.LabelValues(ctx, params.AttributeKey, matchers, time.Time{}, time.Time{})

0 commit comments

Comments
 (0)