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] Configuration breaking changes due to metrics data streams addition #2987

Closed
Tracked by #490
endorama opened this issue Apr 4, 2022 · 12 comments
Closed
Tracked by #490
Labels
Integration:gcp Google Cloud Platform Stalled Team:Cloud-Monitoring Label for the Cloud Monitoring team

Comments

@endorama
Copy link
Member

endorama commented Apr 4, 2022

This issue was noticed while working on #490; it's goal is to migrate gcp beats metricsets to gcp package data streams, allowing the Agent to ingest data related to GCP metrics.

Context

Issue

Given the missing support for Policy Template level variables in Fleet UI, some of the configuration variables must be moved: some configuration are shared by all data streams (logs and metrics one), like credentials and GCP project ID, so it make sense to move them to Package level variables and some are only pertaining to logs data streams, so it make sense to move them within the data streams themselves.

This introduce a configuration breaking change. (Thanks @andrewkroh for raising this concern)

Testing

In order to verify the breaking change and test the upgrade path, I bumped the gcp package with changes in #2707 to the next major (these changes are not yet pushed to #2707).

These were the steps:

  1. Start a test cluster with 8.2.0-SNAPSHOT
    $ elastic-package stack up --version "8.2.0-SNAPSHOT" -d -v
  2. Install latest gcp package (1.5.0 at this moment) through Integrations page
    a. Set Project ID and Credentials JSON to non-default values
    b. Create a new agent policy with the integration
    c. Configuration screenshot
  3. Build the gcp 2.0.2 package from https://github.com/endorama/integrations/tree/gcp-metrics-II
    $ cd packages/gcp && elastic-package build
  4. Restart the local package registry
    $ elastic-package stack up --services package-registry -d
  5. Upgrade the installed integration
    a. click on the upgrade button in the gcp integration "Settings" page
    b. from gcp integration "Integration policies" page click on upgrade button for the defined policy
    c. The upgrade steps identifies some conflicting fields.
    Note: in this screen Project ID, Credentials file and Credentials JSON fields are missing Full page screenshot
    d. Add foobar to GCP Billing Metrics Dataset ID field and click "Upgrade integration`
  6. Verify if the issue discovered in 5.c persists; it does, see screenshot (opacity on bottom bar added for clarity)

Conclusions

Suggested course of action

Considering that:

  • introducing this breaking change with the current upgrade path is problematic for our users
  • this blocks adding metrics related data streams to gcp package
  • there are some metrics data streams that could be released quickly in beta

my suggested course of action would be:

  1. Do not introduce breaking changes in gcp package and create a new gcp_metrics package with metrics related data stream.
  2. Engage with Fleet UI team to solve the issue.
  3. Continue migration from metricsets to data streams in gcp_metrics package.
  4. Merge gcp and gcp_metrics at a later date.

/cc @andrewkroh @ravikesarwani @masci @elastic/obs-cloud-monitoring

@masci
Copy link

masci commented Apr 4, 2022

I'm +1 with point 1., we have the precedent of azure metrics showing this is the best course of action, to the point that I wouldn't even do 4.

@kaiyan-sheng
Copy link
Contributor

+1 on Massi's comment. I think having gcp_metrics can be considered as a permanent solution instead of just a workaround.

@ravikesarwani
Copy link

While my personal preference would be a single package like AWS we cannot afford a breaking change since we have things like GCP dataflow templates based ingestion as well.

Did we look at all the way to see if we can keep it in a single package (gcp) but not introduce a breaking change?
cc: @ruflin to see if there are any extra suggestion here.

If there's no way to add metrics to the existing gcp package then I think we will need to go with the separate gcp_metrics package as proposed and +1 by Massi and Kaiyan.

@ruflin
Copy link
Contributor

ruflin commented Apr 5, 2022

@joshdover Could you take a look at this from a Fleet perspective on what we could do here?

@andrewkroh
Copy link
Member

I have the same opinion as Ravi. We need to avoid a breaking change and it would be nice to keep it in a single package.

I think this growth path for existing packages will be common. Packages may need to create more policy_templates or shift variables from global scope to policy_template scope and it would be good if upgrades continue to work. Maybe we should talk through some common update "use cases" that we want to handle and see what it would take to handle them in Fleet.

@endorama
Copy link
Member Author

endorama commented Apr 7, 2022

We decided to put taking a decision about this on hold, while I'm investigating with Fleet team the timeline for a fix to the mentioned Kibana issue.

@endorama
Copy link
Member Author

Based on the discuss we had in Cloud Integrations weekly meeting we are holding on this decision until the Fleet UI team provides an estimation for the fix detailed in the linked Kibana issue.

@endorama
Copy link
Member Author

Reporting here the answer from Fleet UI team:

If we were to add this support, it would probably take 2 weeks or so, but it is currently not on our foreseeable roadmap due to other product priorities that are higher.

@endorama
Copy link
Member Author

After today Cloud Integrations weekly meeting we reached this conclusion: we are going to move forward with a separate package.

This is due to 2 main elements:

This issue will not be closed as the breaking change is still an issue that need to be addressed in the future when merging gcp and gcp_metrics packages (something that we want to do to provide a consistent user experience across all 3 major cloud providers), but our decision removes this as a blocker for #490

/cc @ruflin @andrewkroh @joshdover

@endorama endorama added the Team:Cloud-Monitoring Label for the Cloud Monitoring team label Apr 26, 2022
@endorama endorama changed the title [GCP] Configuration breaking changes due adding metrics data streams [GCP] Configuration breaking changes due to metrics data streams addition Apr 29, 2022
@endorama
Copy link
Member Author

The related Kibana issue has been closed, which means is possible to test the upgrade path for the integration with breaking change and expect the issue found to be solved.

@endorama
Copy link
Member Author

This issue is solved in 8.x release track, still present in 7.17 as elastic/kibana#132068 has not yet been backported.

Will close once this is solved also for 7.17

@botelastic
Copy link

botelastic bot commented Sep 8, 2023

Hi! We just realized that we haven't looked into this issue in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:gcp Google Cloud Platform Stalled Team:Cloud-Monitoring Label for the Cloud Monitoring team
Projects
None yet
Development

No branches or pull requests

6 participants