Conversation
Contributor
Author
|
As of writing, the diff between v0 and v1 is: |
This was prompted by charmcraft pack failing with "error: can't find Rust compiler".
Comment on lines
+412
to
+418
| def _decode_content(encoded_content: str) -> str: | ||
| try: | ||
| # Assuming content is encoded, try decoding it. | ||
| return lzma.decompress(base64.b64decode(encoded_content.encode("utf-8"))).decode() | ||
| except (binascii.Error, lzma.LZMAError): | ||
| # Failed to base64-decode or decompress, so probably not encoded. Return as is. | ||
| return encoded_content |
Contributor
There was a problem hiding this comment.
Since we are assuming the content is encoded, shouldn't we split this except so we can log meaningful messages to juju debug-log in case our assumption is False?
| RELATION_INTERFACE_NAME = "prometheus_scrape" | ||
|
|
||
| DEFAULT_ALERT_RULES_RELATIVE_PATH = "./src/prometheus_alert_rules" | ||
|
|
Contributor
There was a problem hiding this comment.
Suggested change
| ENCODING = "utf-8" |
|
|
||
| def _encode_content(content: Union[str, bytes]) -> str: | ||
| if isinstance(content, str): | ||
| content = bytes(content, "utf-8") |
Contributor
There was a problem hiding this comment.
Suggested change
| content = bytes(content, "utf-8") | |
| content = bytes(content, ENCODING) |
| if isinstance(content, str): | ||
| content = bytes(content, "utf-8") | ||
|
|
||
| return base64.b64encode(lzma.compress(content)).decode("utf-8") |
Contributor
There was a problem hiding this comment.
Suggested change
| return base64.b64encode(lzma.compress(content)).decode("utf-8") | |
| return base64.b64encode(lzma.compress(content)).decode(ENCODING) |
| def _decode_content(encoded_content: str) -> str: | ||
| try: | ||
| # Assuming content is encoded, try decoding it. | ||
| return lzma.decompress(base64.b64decode(encoded_content.encode("utf-8"))).decode() |
Contributor
There was a problem hiding this comment.
Suggested change
| return lzma.decompress(base64.b64decode(encoded_content.encode("utf-8"))).decode() | |
| return lzma.decompress(base64.b64decode(encoded_content.encode(ENCODING))).decode() |
|
|
||
| def _exec(self, cmd) -> str: | ||
| result = subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) | ||
| return result.stdout.decode("utf-8").strip() |
Contributor
There was a problem hiding this comment.
Suggested change
| return result.stdout.decode("utf-8").strip() | |
| return result.stdout.decode(ENCODING).strip() |
Abuelodelanada
marked this pull request as ready for review
July 25, 2024 15:15
Abuelodelanada
marked this pull request as draft
July 25, 2024 15:15
Contributor
|
We should probably not move forward with this. Currently, we only compress dashboards, and for now it's probably better to keep everything in plaintext, so we don't fracture the way we handle relation data across our charms. |
Contributor
Author
|
Superseded by #864. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Some productions deployments have several megs of relation data just for alert rules, scrape jobs.
Contributing factors to the high volume of reldata:
As a result, every event cos-proxy gets from nrpe relation-changed, results in reading/writing several megs of data, taking several seconds to complete. When many units are at play, model settling takes a long time.
In addition, relation data limit is 16M, and in common envs we already approach 25% of that.
Solution
TODO:
Context
Testing Instructions
Upgrade Notes