Skip to content

Conversation

@algchoo
Copy link
Contributor

@algchoo algchoo commented Aug 12, 2024

Description

The expression for logs required a label called filename with a value that equates to an alert log file path. In k8s this file is not something that is read from. Requiring an arbitrary label with a value to a path that does not exist is potentially confusing.

The solution, alter the expression to accept filename=pathToAlertLogs or log_type="oracledb" and in the k8s plugin snippets for collecting logs, we add log_type="oracledb" and users can just copy/paste the config without potentially becoming confused.

Important note:

The OracleDB container logs are more than just logs from the alert file and there's not a clear way to filter on just the ones we'd get from the alert log.

Changes

Copy link
Contributor

@Caleb-Hurshman Caleb-Hurshman left a comment

Choose a reason for hiding this comment

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

I haven't been paying close attention to what led to this, but these changes make sense to me based on what I do know ✅

@algchoo algchoo force-pushed the feat/oracledb-logs-expression branch from 5a60ca8 to de111fa Compare August 13, 2024 13:58
@algchoo algchoo marked this pull request as ready for review August 13, 2024 15:19
@algchoo algchoo requested a review from a team as a code owner August 13, 2024 15:19
Copy link
Member

@Dasomeone Dasomeone left a comment

Choose a reason for hiding this comment

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

LGTM, but re: your important note can you please create an issue in the repo to track that in case we need to act on it later? Just as a known potential issue

Edit: Nvm I'll create it :)

@Dasomeone Dasomeone merged commit af84109 into grafana:master Aug 13, 2024
algchoo added a commit to observIQ/jsonnet-libs that referenced this pull request Aug 13, 2024
* update log expression to match on filename or log_type labels

* update log_type to match on oracledb-alert

* revert oracledb-alert to oracledb for log_type
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