Skip to content

Commit

Permalink
Update instance Event schema to include "conversion" flag
Browse files Browse the repository at this point in the history
Metrics gathered from prometheus or CostMgmt will parse/populate the flag to be used during product tag lookup when processing instance data
  • Loading branch information
lindseyburnett committed Apr 22, 2024
1 parent 4211772 commit 9ccc9fa
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
@Component
public class CapacityProductExtractor {

public Set<String> getProducts(Collection<String> engProductIds) {
public Set<String> getProducts(Collection<String> engProductIds, boolean isMigrationOffering) {

Set<String> ignoredSubscriptionIds =
engProductIds.stream()
Expand All @@ -47,10 +47,7 @@ public Set<String> getProducts(Collection<String> engProductIds) {

for (String engProductId : engProductIds) {

// TODO
Boolean is3rdPartyMigration = false;

Variant.findByEngProductId(engProductId, is3rdPartyMigration)
Variant.findByEngProductId(engProductId, isMigrationOffering)
.filter(variant -> !ignoredSubscriptionIds.contains(variant.getSubscription().getId()))
.ifPresent(matches::add);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ private Optional<SubscriptionMeasurementKey> upsertMeasurement(
private void reconcileSubscriptionProductIds(Subscription subscription) {
Offering offering = subscription.getOffering();

Set<String> expectedProducts = productExtractor.getProducts(offering.getProductIdsAsStrings());
Set<String> expectedProducts =
productExtractor.getProducts(
offering.getProductIdsAsStrings(), offering.isMigrationOffering());
var toBeRemoved =
subscription.getSubscriptionProductIds().stream()
.filter(p -> !expectedProducts.contains(p))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,9 @@ private void enrichServiceInstanceFromIncomingFeed(Event event) {
// flow, and we don't have a way to distinguish between payg and non-payg through events.
String role = Optional.ofNullable(event.getRole()).map(Object::toString).orElse(null);

// TODO
boolean is3rdPartyMigrated = false;
event.setProductTag(
SubscriptionDefinition.getAllProductTagsWithPaygEligibleByRoleOrEngIds(
role, event.getProductIds(), null, is3rdPartyMigrated));
role, event.getProductIds(), null, event.getConversion()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,8 @@ private Set<String> getProductTag(Event event) {
// remain old logic
String role = Optional.ofNullable(event.getRole()).map(Object::toString).orElse(null);

// TODO
boolean is3rdPartyMigrated = false;

return SubscriptionDefinition.getAllProductTagsWithPaygEligibleByRoleOrEngIds(
role, event.getProductIds(), null, is3rdPartyMigrated);
role, event.getProductIds(), null, event.getConversion());
}

private Set<String> getBillingAccountIds(String billingAcctId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void setup() {
@MethodSource("generateProductIdMappings")
void testSwatchSubscriptionProductIdsMappings(List<String> engProductId, Set<String> tagNames) {

var actual = extractor.getProducts(engProductId);
var actual = extractor.getProducts(engProductId, false);

assertEquals(tagNames, actual);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ void shouldAddNewMeasurementsAndProductIdsIfNotAlreadyExisting() {
newSubscription.setOffering(offering);

when(denylist.productIdMatches(any())).thenReturn(false);
when(capacityProductExtractor.getProducts(offering.getProductIdsAsStrings()))
when(capacityProductExtractor.getProducts(
offering.getProductIdsAsStrings(), offering.isMigrationOffering()))
.thenReturn(new HashSet<>(productIds));

capacityReconciliationController.reconcileCapacityForSubscription(newSubscription);
Expand Down Expand Up @@ -149,7 +150,8 @@ void shouldUpdateCapacitiesIfThereAreChanges() {
updatedSubscription.setOffering(updatedOffering);

when(denylist.productIdMatches(any())).thenReturn(false);
when(capacityProductExtractor.getProducts(updatedOffering.getProductIdsAsStrings()))
when(capacityProductExtractor.getProducts(
updatedOffering.getProductIdsAsStrings(), updatedOffering.isMigrationOffering()))
.thenReturn(productIds);

capacityReconciliationController.reconcileCapacityForSubscription(updatedSubscription);
Expand Down Expand Up @@ -184,7 +186,8 @@ void shouldRemoveAllCapacitiesWhenProductIsOnDenylist() {
updatedSubscription.setOffering(updatedOffering);

when(denylist.productIdMatches(any())).thenReturn(true);
when(capacityProductExtractor.getProducts(updatedOffering.getProductIdsAsStrings()))
when(capacityProductExtractor.getProducts(
updatedOffering.getProductIdsAsStrings(), updatedOffering.isMigrationOffering()))
.thenReturn(productIds);

capacityReconciliationController.reconcileCapacityForSubscription(updatedSubscription);
Expand Down Expand Up @@ -214,7 +217,8 @@ void shouldAddNewCapacitiesAndRemoveAllStaleCapacities() {
subscription.setOffering(offering);

when(denylist.productIdMatches(any())).thenReturn(false);
when(capacityProductExtractor.getProducts(offering.getProductIdsAsStrings()))
when(capacityProductExtractor.getProducts(
offering.getProductIdsAsStrings(), offering.isMigrationOffering()))
.thenReturn(productIds);

capacityReconciliationController.reconcileCapacityForSubscription(subscription);
Expand Down Expand Up @@ -270,7 +274,8 @@ void shouldNotAttemptToCreateDuplicateMeasurementsWhenNoChanges() {
subscription.setOffering(offering);

when(denylist.productIdMatches(any())).thenReturn(false);
when(capacityProductExtractor.getProducts(offering.getProductIdsAsStrings()))
when(capacityProductExtractor.getProducts(
offering.getProductIdsAsStrings(), offering.isMigrationOffering()))
.thenReturn(productIds);

capacityReconciliationController.reconcileCapacityForSubscription(subscription);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ private void createEventFromDataAndSend(
billingAccountId = labels.get("billing_marketplace_account");
}

boolean is3rdPartyMigrated = Boolean.parseBoolean(labels.get("conversion_success"));

// For the openshift metrics, we expect our results to be a 'matrix'
// vector [(instant_time,value), ...] so we only look at the result's
// getValues() data.
Expand Down Expand Up @@ -288,7 +290,8 @@ private void createEventFromDataAndSend(
productTag,
meteringBatchId,
productIds,
displayName);
displayName,
is3rdPartyMigrated);
// Send if and only if it has not been sent yet.
// Related to https://github.com/RedHatInsights/rhsm-subscriptions/pull/374.
if (eventsSent.add(EventKey.fromEvent(event))) {
Expand Down Expand Up @@ -331,7 +334,8 @@ private Event createOrUpdateEvent(
String productTag,
UUID meteringBatchId,
List<String> productIds,
String displayName) {
String displayName,
boolean is3rdPartyMigrated) {
Event event = new Event();
MeteringEventFactory.updateMetricEvent(
event,
Expand All @@ -351,7 +355,8 @@ private Event createOrUpdateEvent(
productTag,
meteringBatchId,
productIds,
displayName);
displayName,
is3rdPartyMigrated);
return event;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public static Event createMetricEvent(
String productTag,
UUID meteringBatchId,
List<String> productIds,
String displayName) {
String displayName,
boolean is3rdPartyMigrated) {
Event event = new Event();
updateMetricEvent(
event,
Expand All @@ -97,7 +98,8 @@ public static Event createMetricEvent(
productTag,
meteringBatchId,
productIds,
displayName);
displayName,
is3rdPartyMigrated);
return event;
}

Expand Down Expand Up @@ -149,7 +151,8 @@ public static void updateMetricEvent(
String productTag,
UUID meteringBatchId,
List<String> productIds,
String displayName) {
String displayName,
boolean is3rdPartyMigrated) {
toUpdate
.withServiceType(serviceType)
.withTimestamp(measuredTime)
Expand All @@ -172,7 +175,8 @@ public static void updateMetricEvent(
.withInstanceId(instanceId)
.withMeteringBatchId(meteringBatchId)
.withProductTag(Set.of(productTag))
.withProductIds(productIds);
.withProductIds(productIds)
.withConversion(is3rdPartyMigrated);
}

public static String getEventType(String metricId, String productTag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class PrometheusMeteringControllerTest {
private final MetricId expectedMetricId = MetricIdUtils.getCores();
private final String expectedProductTag = "OpenShift-metrics";
private final UUID expectedSpanId = UUID.randomUUID();
private final boolean expected3rdPartyMigrationFlag = false;

@Inject PrometheusMeteringController controller;

@Inject MetricProperties metricProperties;
Expand Down Expand Up @@ -227,7 +229,8 @@ void testCollectOpenShiftMetricsWillPersistEvents() {
expectedProductTag,
expectedSpanId,
List.of(),
expectedDisplayName),
expectedDisplayName,
expected3rdPartyMigrationFlag),
MeteringEventFactory.createMetricEvent(
expectedOrgId,
expectedClusterId,
Expand All @@ -245,7 +248,8 @@ void testCollectOpenShiftMetricsWillPersistEvents() {
expectedProductTag,
expectedSpanId,
List.of(),
expectedDisplayName),
expectedDisplayName,
expected3rdPartyMigrationFlag),
MeteringEventFactory.createCleanUpEvent(
expectedOrgId,
getEventType(expectedMetricId.toString(), expectedProductTag),
Expand Down Expand Up @@ -302,7 +306,8 @@ void testVerifyExistingEventsAreUpdatedWhenReportedByPrometheusAndDeletedIfStale
expectedProductTag,
expectedSpanId,
List.of(),
expectedDisplayName);
expectedDisplayName,
expected3rdPartyMigrationFlag);

List<BaseEvent> expectedEvents =
List.of(
Expand All @@ -324,7 +329,8 @@ void testVerifyExistingEventsAreUpdatedWhenReportedByPrometheusAndDeletedIfStale
expectedProductTag,
expectedSpanId,
List.of(),
expectedDisplayName),
expectedDisplayName,
expected3rdPartyMigrationFlag),
MeteringEventFactory.createCleanUpEvent(
expectedOrgId,
getEventType(expectedMetricId.toString(), expectedProductTag),
Expand Down Expand Up @@ -389,7 +395,8 @@ void testVerifyConflictingSlaCausesSavesFirstValue() {
expectedProductTag,
expectedSpanId,
List.of(),
expectedClusterId);
expectedClusterId,
expected3rdPartyMigrationFlag);

List<BaseEvent> expectedEvents =
List.of(
Expand Down Expand Up @@ -454,7 +461,8 @@ void testCollectAzureOpenShiftMetricsWillPersistCorrectBillingAccountId() {
expectedProductTag,
expectedSpanId,
List.of(),
expectedClusterId),
expectedClusterId,
expected3rdPartyMigrationFlag),
MeteringEventFactory.createMetricEvent(
expectedOrgId,
expectedClusterId,
Expand All @@ -472,7 +480,8 @@ void testCollectAzureOpenShiftMetricsWillPersistCorrectBillingAccountId() {
expectedProductTag,
expectedSpanId,
List.of(),
expectedClusterId),
expectedClusterId,
expected3rdPartyMigrationFlag),
MeteringEventFactory.createCleanUpEvent(
expectedOrgId,
getEventType(expectedMetricId.toString(), expectedProductTag),
Expand Down
Loading

0 comments on commit 9ccc9fa

Please sign in to comment.