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

✨ Added support to run loki queries against openshift loki installation ✨ #11

Closed
wants to merge 3 commits into from

Conversation

seepgoel
Copy link
Contributor

Summary

The existing Loki query script works on the Kind deployment which uses dev version of Loki. However, Loki production deployment on openshift needed some changes to the script to work, such as:

  1. Openshift version needs TLS cert and key.
  2. Some of the labels such as pod, nodename etc, are different in openshift. For pod we have kubernetes_pod_name, for container we have kubernetes_container_name.
  3. Openshift Loki version uses X-Scope-OrgID header to be set to either application, infrastructure or audit.

Related issue(s)

Fixes #
This PR adds a new variable LOKI_INSTALL_TYPE which can be either openshift or dev. And based on LOKI_INSTALL_TYPE constructs a proper request to be sent to loki.

@seepgoel seepgoel changed the title Added support to run loki queries against openshift loki installation ✨ ✨ Added support to run loki queries against openshift loki installation ✨ Jun 13, 2024

log_type := os.Getenv("LOG_TYPE")
tls_cert_file := os.Getenv("TLS_CERT_FILE")
tls_key_file := os.Getenv("TLS_KEY_FILE")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Seep-Goel this looks good, could you perhaps add some comment on how to retrieve or generate the required TLS_CERT_FILE and `TLS_KEY_FILE' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will add comments in the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pdettori ...this has been addressed in my latest commit. I'm happy to incorporate any additional feedback you have on the PR.

Copy link
Collaborator

@pdettori pdettori left a comment

Choose a reason for hiding this comment

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

@seepgoel thanks, this looks good enough to merge, could you please amend your commits to pass the DCO Check ?

Signed-off-by: Seep Goel <[email protected]>
@seepgoel seepgoel closed this by deleting the head repository Jun 26, 2024
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.

3 participants