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

[GCP] Fix loadbalancing_metrics prefix #7287

Merged
merged 7 commits into from Aug 14, 2023

Conversation

gpop63
Copy link
Contributor

@gpop63 gpop63 commented Aug 7, 2023

What does this PR do?

Loadbalancing dashboards are using gcp.loadbalancing prefix but they need to use gcp.loadbalancing_metrics since the pipeline is using this naming. The fields prefix in fields.yml is also wrong.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

localhost_5601_app_dashboards (1)

localhost_5601_app_dashboards

@gpop63 gpop63 self-assigned this Aug 7, 2023
@elasticmachine
Copy link

elasticmachine commented Aug 7, 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-08-10T08:05:19.251+0000

  • Duration: 20 min 36 sec

Test stats 🧪

Test Results
Failed 0
Passed 65
Skipped 0
Total 65

🤖 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

elasticmachine commented Aug 7, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (6/6) 💚
Files 100.0% (6/6) 💚
Classes 100.0% (6/6) 💚
Methods 87.826% (101/115) 👎 -12.174
Lines 96.0% (1464/1525) 👍 0.259
Conditionals 100.0% (0/0) 💚

@gpop63 gpop63 marked this pull request as ready for review August 7, 2023 12:08
@gpop63 gpop63 requested review from a team as code owners August 7, 2023 12:08
@gpop63 gpop63 assigned gpop63 and unassigned gpop63 Aug 8, 2023
@@ -1,4 +1,9 @@
# newer versions go on top
- version: "2.24.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought : Should this be a minor version bump as there is a change in field name. cc : @andrewkroh

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this should be a minor version bump

@ishleenk17
Copy link
Contributor

@gpop63 What will be the impact on the users using this integration, when they upgrade?
Field names will change for them. This would be a breaking change for them.

@gpop63 gpop63 requested a review from a team August 10, 2023 08:19
@gpop63
Copy link
Contributor Author

gpop63 commented Aug 10, 2023

@ishleenk17 won't be a breaking change since fields were still using the correct prefix because of the pipeline (I'll look more into it).

This is before changing fields.yml:

"gcp": {
  "loadbalancing_metrics": {
    "https": {
      "response": {
        "bytes": 893
      }
    }
  },

After the change:

"gcp": {
  "loadbalancing_metrics": {
    "https": {
      "backend_request": {
        "count": 1
      }
    }
  },

@gpop63 gpop63 merged commit 8705086 into elastic:main Aug 14, 2023
4 checks passed
@elasticmachine
Copy link

Package gcp - 2.26.0 containing this change is available at https://epr.elastic.co/search?package=gcp

gizas pushed a commit that referenced this pull request Sep 5, 2023
* fix loadbalancing_metrics fields prefix

* bump package version

* fix PR number in changelog

* fix fields in loadbalancing dashboards

* fix sample_event

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

Successfully merging this pull request may close these issues.

[O11y][GCP] Update fields in GCP Load balancing dashboards
5 participants