Skip to content

Commit 447c11b

Browse files
committed
Bug 1822396: Update metric when Subscription is updated
In #1519, the subscription_sync_total metric was updated when the related subscription object was updated. However, the PR had a bug, in which the map used to store metric related information was not being updated when the metric was udpated. As a result of which any update after the first update was not removing the old metric. This PR fixes that bug.
1 parent afd2348 commit 447c11b

File tree

3 files changed

+59
-9
lines changed

3 files changed

+59
-9
lines changed

pkg/metrics/metrics.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,5 +267,7 @@ func UpdateSubsSyncCounterStorage(sub *olmv1alpha1.Subscription) {
267267
counterValues.installedCSV = sub.Status.InstalledCSV
268268
counterValues.pkg = sub.Spec.Package
269269
counterValues.channel = sub.Spec.Channel
270+
271+
subscriptionSyncCounters[sub.GetName()] = counterValues
270272
}
271273
}

test/e2e/like_metric_matcher_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ func WithLabel(n, v string) MetricPredicate {
4747
}
4848
}
4949

50+
func WithName(name string) MetricPredicate {
51+
return WithLabel("name", name)
52+
}
53+
54+
func WithChannel(channel string) MetricPredicate {
55+
return WithLabel("channel", channel)
56+
}
57+
58+
func WithPackage(pkg string) MetricPredicate {
59+
return WithLabel("package", pkg)
60+
}
61+
5062
func WithValue(v float64) MetricPredicate {
5163
return MetricPredicate{
5264
name: fmt.Sprintf("WithValue(%g)", v),

test/e2e/metrics_e2e_test.go

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,9 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
133133
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
134134
}).Should(ContainElement(LikeMetric(
135135
WithFamily("subscription_sync_total"),
136-
WithLabel("name", "metric-subscription-for-create"),
137-
WithLabel("channel", stableChannel),
138-
WithLabel("package", testPackageName),
136+
WithName("metric-subscription-for-create"),
137+
WithChannel(stableChannel),
138+
WithPackage(testPackageName),
139139
)))
140140
})
141141
})
@@ -152,7 +152,7 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
152152
if err != nil {
153153
return err
154154
}
155-
s.Spec.Channel = "beta"
155+
s.Spec.Channel = betaChannel
156156
_, err = crc.OperatorsV1alpha1().Subscriptions(s.GetNamespace()).Update(context.TODO(), s, metav1.UpdateOptions{})
157157
return err
158158
}).Should(Succeed())
@@ -170,17 +170,53 @@ var _ = Describe("Metrics are generated for OLM managed resources", func() {
170170
}).Should(And(
171171
Not(ContainElement(LikeMetric(
172172
WithFamily("subscription_sync_total"),
173-
WithLabel("name", "metric-subscription-for-update"),
174-
WithLabel("channel", stableChannel),
173+
WithName("metric-subscription-for-update"),
174+
WithChannel(stableChannel),
175175
))),
176176
ContainElement(LikeMetric(
177177
WithFamily("subscription_sync_total"),
178-
WithLabel("name", "metric-subscription-for-update"),
179-
WithLabel("channel", "beta"),
180-
WithLabel("package", testPackageName),
178+
WithName("metric-subscription-for-update"),
179+
WithChannel(betaChannel),
180+
WithPackage(testPackageName),
181181
)),
182182
))
183183
})
184+
When("The subscription object is updated again", func() {
185+
186+
BeforeEach(func() {
187+
Eventually(func() error {
188+
s, err := crc.OperatorsV1alpha1().Subscriptions(subscription.GetNamespace()).Get(context.TODO(), subscription.GetName(), metav1.GetOptions{})
189+
if err != nil {
190+
return err
191+
}
192+
s.Spec.Channel = alphaChannel
193+
_, err = crc.OperatorsV1alpha1().Subscriptions(s.GetNamespace()).Update(context.TODO(), s, metav1.UpdateOptions{})
194+
return err
195+
}).Should(Succeed())
196+
})
197+
198+
It("deletes the old subscription metric and emits the new metric(there is only one metric for the subscription)", func() {
199+
Eventually(func() []Metric {
200+
return getMetricsFromPod(c, getPodWithLabel(c, "app=catalog-operator"), "8081")
201+
}).Should(And(
202+
Not(ContainElement(LikeMetric(
203+
WithFamily("subscription_sync_total"),
204+
WithName("metric-subscription-for-update"),
205+
WithChannel(stableChannel),
206+
))),
207+
Not(ContainElement(LikeMetric(
208+
WithFamily("subscription_sync_total"),
209+
WithName("metric-subscription-for-update"),
210+
WithChannel(betaChannel),
211+
))),
212+
ContainElement(LikeMetric(
213+
WithFamily("subscription_sync_total"),
214+
WithName("metric-subscription-for-update"),
215+
WithChannel(alphaChannel),
216+
WithPackage(testPackageName),
217+
))))
218+
})
219+
})
184220
})
185221

186222
When("A subscription object is deleted after emitting metrics", func() {

0 commit comments

Comments
 (0)