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

Add support for pebble log forwarding #332

Merged
merged 39 commits into from
Jan 25, 2024
Merged

Conversation

lucabello
Copy link
Contributor

@lucabello lucabello commented Jan 12, 2024

Issue

Closes #333.

Solution

The solution has changed so many times, you'll probably have a better understanding by reading the documentation additions in the PR.

@lucabello
Copy link
Contributor Author

lucabello commented Jan 16, 2024

TODO: make sure that the class is observing logging relation (or relation_name) changed and if there's a loki endpoint it should enable log forwarding automatically.

A client charm that instantiate this and relates to Loki directly, should be able to only call the constructor and have things work out of the box.

@IbraAoad
Copy link
Contributor

Unit tests are still missing validations of the actual pebble layer due to this canonical/operator#1111

@lucabello lucabello marked this pull request as ready for review January 18, 2024 12:39
@lucabello lucabello changed the title add draft skeleton for pebble log forwarding Add support for pebble log forwarding Jan 18, 2024
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

I see some issues with the API and a bunch of improvements that can be made on the documentation side. Take a look and let me know!

lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
Copy link
Contributor

@PietroPasotti PietroPasotti left a comment

Choose a reason for hiding this comment

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

Still some unclarity left in the updated API.

lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Outdated Show resolved Hide resolved
@Abuelodelanada
Copy link
Contributor

Shouldn't we have itest that verifies this feature?

For instance would be nice to have an itest that checks if you have a charm with 2 containers everything works

@lucabello lucabello merged commit 6ceebb0 into main Jan 25, 2024
13 checks passed
@lucabello lucabello deleted the feature/pebble-log-forwarding branch January 25, 2024 15:39
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.

Add Pebble log forwarding support in loki_push_api
5 participants