Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SWATCH-2302: Capture AWS message transmission statuses in micrometer #3957

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

awood
Copy link
Contributor

@awood awood commented Nov 18, 2024

Jira issue: SWATCH-2302

Description

When usage is processed by swatch-producer-aws, it increments a counter named
"swatch_producer_metered_total" with the amount in metric units, labeled with

  • product (swatch product tag)
  • metric_id (metric ID)
  • billing_provider
  • status (failed/succeeded/duplicate)

Testing

Unit tests are in the PR.

Setup

  1. SERVER_PORT=8001 QUARKUS_MANAGEMENT_PORT=9001 ./gradlew :swatch-producer-aws:quarkusDev
    

Steps

  1. Verify the counters are zero
    http localhost:9001/metrics | grep swatch_producer_metered_total
    
  2. Produce a message for platform.rhsm-subscriptions.billable-usage-hourly-aggregate. I do it with IntelliJ, but use whatever tool you like.
    {"windowTimestamp":"2023-12-21T01:10:28Z","aggregateId": "e81bc918-6434-4a58-9192-8a8dd73ec8fb","aggregateKey": {"orgId": "org123","billingProvider": "aws","metricId": "Cores","productId": "rosa"}}
    This will produce an error since there's no AWS to communicate with

Verification

  1. Repeat
    http localhost:9001/metrics | grep swatch_producer_metered_total
    
  2. You'll see
    swatch_producer_metered_total{billing_provider="aws",error_code="usage_context_lookup",metric_id="Cores",product_tag="rosa",status="failed"} 0.0
    
    with the billing_provider, error_code, metric_id, product_id, and status.

@awood awood added QE Pull request should be approved by QE before merge Dev Pull requests that need developer review labels Nov 18, 2024
@awood awood force-pushed the awood/SWATCH-2302-producer-aws-metric branch from b4d79aa to f0dfa61 Compare November 18, 2024 21:40
@@ -20,8 +20,7 @@
*/
package com.redhat.swatch.aws.service;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this was automatically done by intellij, please, avoid diamond imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. But I think we do use wildcards a lot in tests for static imports for JUnit, Mockito, and Hamcrest. I don't think there's an official policy on this enforced by any tooling so it would be good to reach a team consensus on this.

% rg 'import static.*\*' -l | wc -l
83

So 83 classes with wildcards and nearly all of them are in test it looks like. There are a handful in swatch-core and the main package. We should probably either fix these or at least set the style linter to catch them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style guide is unambiguous about forbidding wildcard imports. So, now we just need to figure out why it's not enforcing that or if we want to as a team make an exception for org.junit, org.mockito stuff

@Sgitario Sgitario self-assigned this Nov 19, 2024
@awood awood force-pushed the awood/SWATCH-2302-producer-aws-metric branch from f0dfa61 to 992ff4a Compare November 19, 2024 16:39
@awood awood force-pushed the awood/SWATCH-2302-producer-aws-metric branch from 992ff4a to 7ecfdc9 Compare November 19, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev Pull requests that need developer review QE Pull request should be approved by QE before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants