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

Add new validations to manage object_type and metric_type fields #527

Merged
merged 4 commits into from Jul 18, 2023

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented May 17, 2023

What does this PR do?

This PR adds new validations to include the cases where object_type and/or metric_type are set in field definitions.

  • If a field defines an object_type (new):
    • the only possible value for type is object.
  • If a field defines a metric_type and an object_type
    • currently, aggregate_metric_double not allowed.
    • object_type just could have a different value of possible values:
      • histogram
      • long
      • integer
      • short
      • byte
      • double
      • float
      • half_float
      • scaled_float
      • unsigned_long
  • If a field defines a metric_type but it does not define an object_type
    • type just could have these allowed values:
      • histogram
      • aggregate_metric_double
      • long
      • integer
      • short
      • byte
      • double
      • float
      • half_float
      • scaled_float
      • unsigned_long

This PR allows field definitions like this:

- name: prometheus.*.value
  type: object
  object_type: double
  object_type_mapping_type: "*"
  metric_type: gauge
  descriptoin: Prometheus gauge metric

Previously, it was failing with this error:

  1. file "/home/mariorodriguez/Coding/work/integrations/build/packages/aws-1.46.6.zip/data_stream/cloudwatch_metrics/fields/package-fields.yml" is invalid: field 1.type: 1.type must be one of the following: "histogram", "aggregate_metric_double", "long", "integer", "short", "byte", "double", "float", "half_float", "scaled_float", "unsigned_long"

Why is it important?

This new validation will check that the types of the fields will be the right ones when metric_type is set for a field (gauge or counter). Not all types are valid when this is enabled. Based on this doc

Checklist

  • I have added test packages to test/packages that prove my change is effective.
  • I have added an entry in spec/changelog.yml.
  • Added JSON Patches for previous versions.

Related issues

@mrodm mrodm self-assigned this May 17, 2023
@elasticmachine
Copy link

elasticmachine commented May 17, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-05-17T11:52:24.571+0000

  • Duration: 8 min 49 sec

Test stats 🧪

Test Results
Failed 0
Passed 842
Skipped 0
Total 842

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (8/8) 💚
Files 68.966% (20/29) 👍
Classes 73.171% (30/41) 👍
Methods 56.154% (73/130) 👍
Lines 43.029% (679/1578) 👍
Conditionals 100.0% (0/0) 💚

@mrodm
Copy link
Contributor Author

mrodm commented Jul 13, 2023

test integrations

@elasticmachine
Copy link

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6948

@mrodm
Copy link
Contributor Author

mrodm commented Jul 13, 2023

Latest run with integrations, the following packages have failed their check:

  • azure_application_insights
  • azure_billing
  • hashicorp_vault

These are the errors for each packages:

  • azure_application_insights
[2023-07-13T12:42:38.477Z] Error: checking package failed: linting package failed: found 4 validation errors:
[2023-07-13T12:42:38.477Z]    1. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_application_insights/data_stream/app_insights/fields/package-fields.yml" is invalid: field 0.fields.1.fields.4.type: 0.fields.1.fields.4.type must be one of the following: "object"
[2023-07-13T12:42:38.477Z]    2. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_application_insights/data_stream/app_insights/fields/package-fields.yml" is invalid: field 0.fields.5.type: 0.fields.5.type must be one of the following: "object"
[2023-07-13T12:42:38.477Z]    3. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_application_insights/data_stream/app_state/fields/package-fields.yml" is invalid: field 0.fields.1.fields.4.type: 0.fields.1.fields.4.type must be one of the following: "object"
[2023-07-13T12:42:38.477Z]    4. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_application_insights/data_stream/app_state/fields/package-fields.yml" is invalid: field 0.fields.5.type: 0.fields.5.type must be one of the following: "object"
  • azure_billing
[2023-07-13T12:42:37.010Z] Error: checking package failed: linting package failed: found 2 validation errors:
[2023-07-13T12:42:37.010Z]    1. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_billing/data_stream/billing/fields/package-fields.yml" is invalid: field 0.fields.1.fields.4.type: 0.fields.1.fields.4.type must be one of the following: "object"
[2023-07-13T12:42:37.010Z]    2. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/azure_billing/data_stream/billing/fields/package-fields.yml" is invalid: field 0.fields.5.type: 0.fields.5.type must be one of the following: "object"
  • hashicorp_vault
[2023-07-13T12:40:24.777Z] Error: checking package failed: linting package failed: found 1 validation error:
[2023-07-13T12:40:24.777Z]    1. file "/var/lib/jenkins/workspace/est-manager_integrations_PR-6948/src/github.com/elastic/integrations/packages/hashicorp_vault/data_stream/audit/fields/fields.yml" is invalid: field 0.fields.0.fields.0.fields.10.type: 0.fields.0.fields.0.fields.10.type must be one of the following: "object"

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @mrodm

@mrodm
Copy link
Contributor Author

mrodm commented Jul 17, 2023

test integrations

@elasticmachine
Copy link

Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6948

@mrodm mrodm changed the title Add new checks to manage object_type and metric_type fields Add new validations to manage object_type and metric_type fields Jul 18, 2023
@mrodm
Copy link
Contributor Author

mrodm commented Jul 18, 2023

Adding the JSON Patches to remove these validation in previous versions , packages failing running "test integrations" are not due to elastic-package check -v command failing. And those 3 packages (azure_application_insights, azure_billing and hashicorp_vault) are run successfully.

https://fleet-ci.elastic.co/blue/organizations/jenkins/Ingest-manager%2Fintegrations/detail/PR-6948/3/
https://fleet-ci.elastic.co/blue/organizations/jenkins/Ingest-manager%2Fintegrations/detail/PR-6948/4/

@mrodm mrodm marked this pull request as ready for review July 18, 2023 13:41
@mrodm mrodm requested a review from a team as a code owner July 18, 2023 13:41
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

👍

@mrodm mrodm merged commit 7990807 into elastic:main Jul 18, 2023
3 checks passed
@mrodm mrodm deleted the add_validation_object_type_metric_type branch July 18, 2023 16:23
@tetianakravchenko
Copy link
Contributor

Hey @mrodm when is it planned to get this changes available in elastic-package?

@jsoriano
Copy link
Member

jsoriano commented Aug 7, 2023

Hey @mrodm when is it planned to get this changes available in elastic-package?

We will prepare releases of package-spec and elastic-package today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants