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: opt-in to expose successful connection logs #123

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

therealvio
Copy link

@therealvio therealvio commented Jan 16, 2025

Purpose 🎯

This change adds a configuration option for users to expose successful connection logs. This is controlled via an environment variable that, when set to true, will produce the successful connection log as an info level message, rather than a debug level message.

Exposing these logs can be useful for calculating SLIs. SLIs are the quotient of errors over valid requests, therefore all valid messages need to be surfaced to perform an accurate calculation of the metric.

Context 🧠

Note

#122 should be finalised, and merged before this PR comes in since it is based off of the changes it includes.

This changes removes the addition of the eventPayload key that was
imposed in the previous commit. I would rather leave the decision to the
maintainers.

Whether it be reverting *this* commit, or accepting this one, or leaving
it to the future PR.
This change adds a configuration option for users to expose successful
connection logs. This is controlled via an environment variable that,
when set to true, will produce the successful connection log as an
`info` level message, rather than a `debug` level message.

Exposing these logs can be useful for calculating SLIs. SLIs are the
quotient of errors over valid requests, therefore all valid messages
need to be surfaced to perform an accurate calculation of the metric.
@@ -5,6 +5,7 @@ locals {
: replace(upper(obj.az), "-", "_") => join(",", obj.route_table_ids)
}
has_ipv6_env_var = { "HAS_IPV6" = var.lambda_has_ipv6 }
log_successful_connections_env_var = { "LOG_SUCCESSFUL_CONNECTIONS" = var.log_successful_connections }
Copy link
Author

Choose a reason for hiding this comment

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

I am not incredibly versed in base Terraform, being a CDKTF user myself. I am looking to make sure I am adding this variable right. Any and all feedback on how to achieve this is welcome!

@therealvio therealvio marked this pull request as ready for review January 16, 2025 05:03
@therealvio therealvio requested a review from a team as a code owner January 16, 2025 05:03
@therealvio therealvio changed the title feat: use structured logging with structlog feat: opt-in to expose successful connection logs Jan 16, 2025
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.

1 participant