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

feat!: Add support for Log Analytics and Remove BigQuery log destination #1025

Conversation

daniel-cit
Copy link
Contributor

@daniel-cit daniel-cit commented Nov 13, 2023

This PR adds support for Log Analytics in the Log Bucket destination and removes the BigQuery log destination from the centralized logging configured in step 1-org.

Now that Log Analytics is available in GA, the same use cases for analytics that were previously accomplished using a log sink export to BigQuery can now be accomplished with a log sink export to a Logging bucket.

This change helps address use cases for both writing custom SQL queries for deep analysis analysis and ad-hoc investigation using the Logs Explorer console, with one single log sink and data source instead of two.

For more information on the differences of these log sink types see Moving to Log Analytics for BigQuery export users

@daniel-cit daniel-cit marked this pull request as ready for review November 14, 2023 16:55
@daniel-cit daniel-cit requested review from rjerrems, gtsorbo and a team as code owners November 14, 2023 16:55
Copy link

@prateek2408 prateek2408 left a comment

Choose a reason for hiding this comment

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

Added some comments.

1-org/envs/shared/log_sinks.tf Show resolved Hide resolved
1-org/envs/shared/log_sinks.tf Show resolved Hide resolved
1-org/modules/centralized-logging/README.md Show resolved Hide resolved
test/integration/org/org_test.go Show resolved Hide resolved
Copy link

@jasonbisson jasonbisson left a comment

Choose a reason for hiding this comment

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

The only concern I had the change would trigger a removal of the BQ dataset, but I only saw the change to the log filter.

@daniel-cit
Copy link
Contributor Author

The only concern I had the change would trigger a removal of the BQ dataset, but I only saw the change to the log filter.

@jasonbisson
the new recommendation is to use log analytics with the log Bucket creating a Big query dataset associated with the log bucket.

This change removed the direct export to a BigQuery dataset that existed previously from the main code and from the inner module we use.

If this new version would be applied over a deployed version of the master branch it would remove an existing BigQuery dataset.

This is the reason for the feat!: to indicate a braking change in the code.

There is no in-place upgrade path from the a version of the foundation to a new version .

Copy link
Collaborator

@gtsorbo gtsorbo left a comment

Choose a reason for hiding this comment

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

LGTM

@daniel-cit
Copy link
Contributor Author

@gtsorbo @apeabody @bharathkkb build is green :)

@apeabody apeabody merged commit 25c61c4 into terraform-google-modules:master Dec 13, 2023
5 checks passed
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.

6 participants