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-1629: Include addonSamples as a template for accountQueryTemplate #2485

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

Sgitario
Copy link
Contributor

@Sgitario Sgitario commented Sep 4, 2023

Jira issue: SWATCH-1629

Description

Currently, Rhods contains "addonSamples", but application.yaml does not have a template for it. Therefore, it reads everything by "default" using default query and filters it later in the code.

Now, we have properly configured the "addonSamples" query profile that includes the resource_type and resource_name filters directly in the prometheus query.

Testing

I've verified these changes via:

a. Using the new prometheus promql script:

1.- Run the script:

bin/metering-promql.py --product rhods --url https://grafana.app-sre.devshift.net

2.- It will generate the following output:

Accounts: https://grafana.app-sre.devshift.net/explore?orgId=1&left=%7B%22queries%22%3A+%5B%7B%22refId%22%3A+%22A%22%2C+%22editorMode%22%3A+%22code%22%2C+%22expr%22%3A+%22group%28min_over_time%28ocm_subscription_resource%7Bresource_type%3D%5C%22addon%5C%22%2C+resource_name%3D%27addon-open-data-hub%27%2C+external_organization+%21%3D+%27%27%2C+billing_model%3D%27marketplace%27%7D%5B1h%5D%29%29+by+%28external_organization%29%22%2C+%22legendFormat%22%3A+%22__auto%22%2C+%22range%22%3A+true%2C+%22instant%22%3A+true%2C+%22interval%22%3A+%223600%22%7D%5D%2C+%22range%22%3A+%7B%22from%22%3A+%22now-12h%22%2C+%22to%22%3A+%22now%22%7D%7D
  Cores: https://grafana.app-sre.devshift.net/explore?orgId=1&left=%7B%22queries%22%3A+%5B%7B%22refId%22%3A+%22A%22%2C+%22editorMode%22%3A+%22code%22%2C+%22expr%22%3A+%22cluster%3Ausage%3Aworkload%3Acapacity_virtual_cpu_hours+%2A+on%28_id%29+group_right+min_over_time%28ocm_subscription_resource%7Bresource_type%3D%5C%22addon%5C%22%2Cresource_name%3D%5C%22addon-open-data-hub%5C%22%2C+external_organization%3D~%5C%22.%2A%5C%22%2C+billing_model%3D%5C%22marketplace%5C%22%2C+support%3D~%5C%22Premium%7CStandard%7CSelf-Support%7CNone%5C%22%7D%5B1h%5D%29%22%2C+%22legendFormat%22%3A+%22__auto%22%2C+%22range%22%3A+true%2C+%22instant%22%3A+true%2C+%22interval%22%3A+%223600%22%7D%5D%2C+%22range%22%3A+%7B%22from%22%3A+%22now-12h%22%2C+%22to%22%3A+%22now%22%7D%7D

Checking the Accounts link, you can confirm that the query is now: group(min_over_time(ocm_subscription_resource{resource_type="addon", resource_name='addon-open-data-hub', external_organization != '', billing_model='marketplace'}[1h])) by (external_organization)

Before these changes, the filters were group(min_over_time(ocm_subscription_resource{product='rhods', external_organization != '', billing_model='marketplace'}[1h])) by (external_organization).

b. Using the service

1.- podman-compose up
2.- Mock the prometheus server at localhost:

Create the stub directory:
mkdir -p stub/__files

Add a stub file to return no data: stub/__files/empty.json

cat > stub/__files/empty.json <<EOF
{
    "status" : "success",
    "data" : {
      "resultType" : "matrix",
      "result" : []
    }
}
EOF

Run wiremock:
podman run -it --rm -p 8101:8080 --name wiremock -v $PWD/stub:/home/wiremock:z wiremock/wiremock:2.32.0 --verbose

Configure stubbing for prometheus:

  • this is to return no data otherwise:
    curl -X POST --data '{ "priority": 5, "request": { "urlPath": "/api/v1/query_range", "method": "GET" }, "response": { "status": 200, "bodyFileName": "empty.json", "headers":{"Content-Type":"application/json"} }}' http://localhost:8101/__admin/mappings/new

3.- Start the swatch metrics app: PROM_URL="http://localhost:8101/api/v1/" SPRING_PROFILES_ACTIVE=metering-job,kafka-queue ./gradlew :bootRun

And wait until the app terminates.

5.- Check the prometheus logs (you started the wiremock server using --verbose) and confirm you received:

10.0.2.100 - GET /api/v1/query_range?query=group%28min_over_time%28ocm_subscription_resource%7Bresource_type%3D%22addon%22%2C%20resource_name%3D%27addon-open-data-hub%27%2C%20external_organization%20%21%3D%20%27%27%2C%20billing_model%3D%27marketplace%27%7D%5B1h%5D%29%29%20by%20%28external_organization%29&start=1694073600&end=1694073600&step=3600&timeout=10000

Which now it contains the resource_name=addon-open-data-hub and resource_type=addon.

Before these changes, the query was:

2023-09-07 07:57:27.105 Request received:
10.0.2.100 - GET /api/v1/query_range?query=group%28min_over_time%28ocm_subscription_resource%7Bproduct%3D%27%27%2C%20external_organization%20%21%3D%20%27%27%2C%20billing_model%3D%27marketplace%27%7D%5B1h%5D%29%29%20by%20%28external_organization%29&start=1694070000&end=1694070000&step=3600&timeout=10000

@Sgitario Sgitario added the QE Pull request should be approved by QE before merge label Sep 4, 2023
@Sgitario Sgitario requested review from lindseyburnett and removed request for kartikshahc September 5, 2023 13:11
@lindseyburnett lindseyburnett requested review from kartikshahc and removed request for kartikshahc September 5, 2023 13:11
@kahowell kahowell added QE Unneeded Pull request does not need QE approval and removed QE Pull request should be approved by QE before merge labels Sep 5, 2023
@kahowell
Copy link
Contributor

kahowell commented Sep 6, 2023

podman run -it --rm -p 8101:8080 --name wiremock -v $PWD/stub:/home/wiremock wiremock/wiremock:2.32.0 --verbose

Looks like you run without selinux enforcing?

PSA: https://stopdisablingselinux.com/

Edit: If no, I'm curious how this works otherwise without -v $PWD/stub:/home/wiremock:z 😄

@lindseyburnett
Copy link
Contributor

Test Step corrections/notes

  • mkdir -p stub/__files
  • wiremock volume mount needs :z at the end
  • "api" profile also needed on application startup to make rest request

Copy link
Contributor

@lindseyburnett lindseyburnett left a comment

Choose a reason for hiding this comment

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

The steps listed don't actually have the application exercising the new template you added. The results you got are misleading. The openshift-metering-worker profile actually uses the set of templates found in application-openshift-metering-worker.yaml. Coincidentally, the templates are identical so you got false positive results.

You can verify that the template isn't being used by setting the OPENSHIFT_ENABLED_ACCOUNT_PROMQL environment variable during startup. When you make the http request, you can see that URL wiremock is receiving didn't change to "bananas".

OPENSHIFT_ENABLED_ACCOUNT_PROMQL="bananas" PROM_URL="http://localhost:8101/api/v1/" SUBSCRIPTION_USE_STUB=true USER_USE_STUB=true RHSM_RBAC_USE_STUB=true SUBSCRIPTION_SYNC_ENABLED=true ENABLE_SYNCHRONOUS_OPERATIONS=true SPRING_PROFILES_ACTIVE=openshift-metering-worker,api,kafka-queue ./gradlew :bootRun

The hourly metering job is what exercises the query template defined in application.yaml. You can do this by starting the application with the following profiles.
SPRING_PROFILES_ACTIVE=metering-job,kafka-queue

You can also set OPENSHIFT_ENABLED_ACCOUNT_PROMQL as noted above to see the difference.

TODO

  • Update QueryBuilder to select which query template to use based on swatch-product-configuration.

var templateKey = queryDescriptor.getMetric().getPrometheus().getQueryKey() should do the trick.

  • Ensure all the product configurations that have a prometheus block also have "queryKey" set. Alternatively, you could set the string to "default" in the com.redhat.swatch.configuration.registry.PrometheusMetric class

@ntkathole ntkathole added QE Pull request should be approved by QE before merge and removed QE Unneeded Pull request does not need QE approval labels Sep 7, 2023
@Sgitario
Copy link
Contributor Author

Sgitario commented Sep 7, 2023

Test Step corrections/notes

  • "api" profile also needed on application startup to make rest request

This is odd since I didn't need this step.

@Sgitario
Copy link
Contributor Author

Sgitario commented Sep 7, 2023

podman run -it --rm -p 8101:8080 --name wiremock -v $PWD/stub:/home/wiremock wiremock/wiremock:2.32.0 --verbose

Looks like you run without selinux enforcing?

PSA: https://stopdisablingselinux.com/

Edit: If no, I'm curious how this works otherwise without -v $PWD/stub:/home/wiremock:z 😄

I had to disable for some reason I don't remember now :)

@Sgitario
Copy link
Contributor Author

Sgitario commented Sep 7, 2023

PR updated with the requested changes and description updated as well.
Thanks @lindseyburnett !

Copy link

@TrayvonMcKnight TrayvonMcKnight left a comment

Choose a reason for hiding this comment

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

@Sgitario Sgitario merged commit 63cba9e into RedHatInsights:main Sep 12, 2023
6 checks passed
@Sgitario Sgitario deleted the SWATCH-1629 branch September 12, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QE Pull request should be approved by QE before merge
Projects
None yet
5 participants