Skip to content

Commit dfde7cd

Browse files
leonorfmartinsleonorfmartins
authored
Change to warning if there is no pricing match for RDS (#851)
1 parent beead31 commit dfde7cd

4 files changed

Lines changed: 82 additions & 46 deletions

File tree

‎pkg/aws/client/pricing.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package client
22

33
import (
44
"context"
5-
"fmt"
65
"time"
76

87
"log/slog"
@@ -270,9 +269,13 @@ func (p *pricing) getRDSUnitData(ctx context.Context, instType, region, deployme
270269
return "", err
271270
}
272271

273-
if len(products.PriceList) != 1 {
274-
slog.ErrorContext(ctx, "expected 1 price list, got", "count", len(products.PriceList))
275-
return "", fmt.Errorf("expected 1 price list, got %d", len(products.PriceList))
272+
if len(products.PriceList) == 0 {
273+
slog.WarnContext(ctx, "no price list found for RDS instance", "instanceType", instType, "region", region)
274+
return "", nil
275+
}
276+
if len(products.PriceList) > 1 {
277+
slog.WarnContext(ctx, "ambiguous price list, expected 1 got multiple", "count", len(products.PriceList), "instanceType", instType, "region", region)
278+
return "", nil
276279
}
277280
return products.PriceList[0], nil
278281
}

‎pkg/aws/client/pricing_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ func Test_GetRDSUnitData(t *testing.T) {
337337
wantErr: false,
338338
},
339339
{
340-
name: "multiple prices",
340+
name: "multiple prices - ambiguous match, skips without error",
341341
GetProducts: func(ctx context.Context, input *awsPricing.GetProductsInput, optFns ...func(*awsPricing.Options)) (*awsPricing.GetProductsOutput, error) {
342342
return &awsPricing.GetProductsOutput{
343343
PriceList: []string{
@@ -347,17 +347,17 @@ func Test_GetRDSUnitData(t *testing.T) {
347347
}, nil
348348
},
349349
want: "",
350-
wantErr: true,
350+
wantErr: false,
351351
},
352352
{
353-
name: "empty price list",
353+
name: "empty price list - no match, skips without error",
354354
GetProducts: func(ctx context.Context, input *awsPricing.GetProductsInput, optFns ...func(*awsPricing.Options)) (*awsPricing.GetProductsOutput, error) {
355355
return &awsPricing.GetProductsOutput{
356356
PriceList: []string{},
357357
}, nil
358358
},
359359
want: "",
360-
wantErr: true,
360+
wantErr: false,
361361
},
362362
{
363363
name: "pricing API errors",

‎pkg/aws/rds/rds.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ func (c *Collector) Collect(ctx context.Context, ch chan<- prometheus.Metric) er
124124
logger.Error("error listing rds prices", "error", err)
125125
return err
126126
}
127+
if v == "" {
128+
logger.Warn("no pricing data found for RDS instance, skipping", "instanceType", *instance.DBInstanceClass, "region", region, "engine", *instance.Engine)
129+
continue
130+
}
127131
validatedPrice, err := validateRDSPriceData(ctx, v)
128132
if err != nil {
129133
logger.Error("error validating RDS price data", "error", err)

‎pkg/aws/rds/rds_test.go‎

Lines changed: 67 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,30 @@ func TestMultiOrSingleAZ(t *testing.T) {
111111

112112
func TestCollector_Collect(t *testing.T) {
113113
const cacheKey = "us-east-1-db.t3.medium-mysql-Single-AZ-AWS Outposts"
114+
validPriceJSON := `{
115+
"terms": {
116+
"OnDemand": {
117+
"term1": {
118+
"priceDimensions": {
119+
"dim1": {
120+
"pricePerUnit": {"USD": "0.456"}
121+
}
122+
}
123+
}
124+
}
125+
}
126+
}`
114127
tests := []struct {
115-
name string
116-
ListRDSInstances []rdsTypes.DBInstance
117-
pricingKey string
128+
name string
129+
ListRDSInstances []rdsTypes.DBInstance
130+
pricingKey string
131+
getRDSUnitDataRet string
132+
expectGetRDSUnitData bool
133+
expectMetric bool
134+
initialCache map[string]float64
118135
}{
119136
{
120-
name: "instance without price in cache",
137+
name: "cache hit - uses cached price",
121138
ListRDSInstances: []rdsTypes.DBInstance{{
122139
DBSubnetGroup: &rdsTypes.DBSubnetGroup{
123140
Subnets: []rdsTypes.Subnet{
@@ -130,35 +147,52 @@ func TestCollector_Collect(t *testing.T) {
130147
},
131148
AvailabilityZone: aws.String("us-east-1a"),
132149
DBInstanceClass: aws.String("db.t3.medium"),
133-
Engine: aws.String("postgres"),
150+
Engine: aws.String("mysql"),
134151
DBInstanceIdentifier: aws.String("test-db"),
135152
MultiAZ: aws.Bool(false),
136153
DbiResourceId: aws.String("test-db"),
137154
DBInstanceArn: aws.String("some-arn"),
138155
}},
139-
pricingKey: createPricingKey("us-east-1", "db.t3.medium", "postgres", "Single-AZ", "AWS Region"),
156+
pricingKey: cacheKey,
157+
expectGetRDSUnitData: false,
158+
expectMetric: true,
159+
initialCache: map[string]float64{cacheKey: 0.123},
140160
},
141161
{
142-
name: "instance with price in cache",
162+
name: "cache miss - fetches price from API",
143163
ListRDSInstances: []rdsTypes.DBInstance{{
144-
DBSubnetGroup: &rdsTypes.DBSubnetGroup{
145-
Subnets: []rdsTypes.Subnet{
146-
{
147-
SubnetOutpost: &rdsTypes.Outpost{
148-
Arn: aws.String("some-outpost-arn"),
149-
},
150-
},
151-
},
152-
},
164+
DBSubnetGroup: &rdsTypes.DBSubnetGroup{},
153165
AvailabilityZone: aws.String("us-east-1a"),
154166
DBInstanceClass: aws.String("db.t3.medium"),
155-
Engine: aws.String("mysql"),
167+
Engine: aws.String("postgres"),
156168
DBInstanceIdentifier: aws.String("test-db-2"),
157169
MultiAZ: aws.Bool(false),
158170
DbiResourceId: aws.String("test-db-2"),
159-
DBInstanceArn: aws.String("some-arn"),
171+
DBInstanceArn: aws.String("some-arn-2"),
160172
}},
161-
pricingKey: cacheKey,
173+
pricingKey: createPricingKey("us-east-1", "db.t3.medium", "postgres", "Single-AZ", "AWS Region"),
174+
getRDSUnitDataRet: validPriceJSON,
175+
expectGetRDSUnitData: true,
176+
expectMetric: true,
177+
initialCache: map[string]float64{},
178+
},
179+
{
180+
name: "no pricing data returned for instance - skips without error",
181+
ListRDSInstances: []rdsTypes.DBInstance{{
182+
DBSubnetGroup: &rdsTypes.DBSubnetGroup{},
183+
AvailabilityZone: aws.String("us-east-1a"),
184+
DBInstanceClass: aws.String("db.t3.medium"),
185+
Engine: aws.String("aurora-postgresql"),
186+
DBInstanceIdentifier: aws.String("test-db-3"),
187+
MultiAZ: aws.Bool(false),
188+
DbiResourceId: aws.String("test-db-3"),
189+
DBInstanceArn: aws.String("some-arn-3"),
190+
}},
191+
pricingKey: createPricingKey("us-east-1", "db.t3.medium", "aurora-postgresql", "Single-AZ", "AWS Region"),
192+
getRDSUnitDataRet: "",
193+
expectGetRDSUnitData: true,
194+
expectMetric: false,
195+
initialCache: map[string]float64{},
162196
},
163197
}
164198

@@ -172,27 +206,14 @@ func TestCollector_Collect(t *testing.T) {
172206
Return(tt.ListRDSInstances, nil).
173207
Times(1)
174208

175-
// if the pricing key is not empty, then we expect the GetRDSUnitData method to be called
176-
if tt.pricingKey != "cache-key" {
209+
if tt.expectGetRDSUnitData {
177210
mockClient.EXPECT().GetRDSUnitData(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
178-
Return(`{
179-
"terms": {
180-
"OnDemand": {
181-
"term1": {
182-
"priceDimensions": {
183-
"dim1": {
184-
"pricePerUnit": {"USD": "0.456"}
185-
}
186-
}
187-
}
188-
}
189-
}
190-
}`, nil).
191-
AnyTimes()
211+
Return(tt.getRDSUnitDataRet, nil).
212+
Times(1)
192213
}
193214

194215
c := &Collector{
195-
pricingMap: &pricingMap{pricingMap: map[string]float64{tt.pricingKey: 0.456, cacheKey: 0.123}},
216+
pricingMap: &pricingMap{pricingMap: tt.initialCache},
196217
regions: []types.Region{{RegionName: aws.String("us-east-1")}},
197218
regionMap: map[string]client.Client{"us-east-1": mockClient},
198219
scrapeInterval: time.Minute,
@@ -203,15 +224,23 @@ func TestCollector_Collect(t *testing.T) {
203224
err := c.Collect(t.Context(), ch)
204225
assert.NoError(t, err)
205226

227+
if !tt.expectMetric {
228+
select {
229+
case <-ch:
230+
t.Fatal("expected no metric to be collected")
231+
default:
232+
}
233+
return
234+
}
235+
206236
select {
207237
case metric := <-ch:
208238
metricResult := utils.ReadMetrics(metric)
209239
close(ch)
210-
assert.NoError(t, err)
211240
labels := metricResult.Labels
212241
hourlyPrice, _ := c.pricingMap.Get(tt.pricingKey)
213242
assert.Equal(t, *tt.ListRDSInstances[0].DBInstanceClass, labels["tier"])
214-
assert.Equal(t, *tt.ListRDSInstances[0].DBInstanceIdentifier, labels["id"])
243+
assert.Equal(t, *tt.ListRDSInstances[0].DbiResourceId, labels["id"])
215244
assert.Equal(t, hourlyPrice, metricResult.Value)
216245
default:
217246
t.Fatal("expected a metric to be collected")

0 commit comments

Comments
 (0)