Fix: TF Agent host charm rebuilds COS scrape jobs based on supervisor config.#1027
Fix: TF Agent host charm rebuilds COS scrape jobs based on supervisor config.#1027rene-oromtz merged 1 commit intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1027 +/- ##
=======================================
Coverage 77.21% 77.21%
=======================================
Files 116 116
Lines 12065 12065
Branches 996 996
=======================================
Hits 9316 9316
Misses 2520 2520
Partials 229 229
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
rene-oromtz
left a comment
There was a problem hiding this comment.
LGTM, as far as I remember, Grafana Agent required the relation with Prometheus to actually configure the scrape jobs.
Due to initial TEL deployment without Prometheus, that relation data may never got populated from our charm side... I think the key is actually the refresh_events addition which also makes sense because when we add new agents, we should also be refreshing the scrape_jobs with the new agents config.
55a02a9 to
da8c9a7
Compare
|
@ajzobro, thanks for pointing out potential improvements. |
ajzobro
left a comment
There was a problem hiding this comment.
Please address the issues that can easily be addressed within the scope of the code that was introduced here, including tests for the failure conditions.
CERTTF-1041 now eixsts as a strategy for handling (kicking along) the technical debt this time.
ajzobro
left a comment
There was a problem hiding this comment.
If we write the tests for the error cases we will clearly see the issues in the code.
|
I guess I'm not getting any more tests, huh? |
ajzobro
left a comment
There was a problem hiding this comment.
Agree with creation of 1041 for improved testing and fixing the relied-upon-but-out-of-scope functions.
|
Thanks @ajzobro! Agree that there is lots to improve. At least now you already identify what needs to be done it should be a bit faster to implement in 1041 |
Description
After deploying the Grafana Agent hosts in TEL, the
/etc/grafana-agent.yamlconfig was not properly populated with scrape endpoints.Following investigation, the suspicion is that since
charm.pyis instantiated on every Juju event, the in-memoryself.scrape_endpointsvariable is not consistently populated.To address this,
COSAgentProvider.scrape_configsare now sourced directly from thesupervisordconfiguration.Resolved issues
CERTTF-1038
Documentation
Charm unit tests.
Web service API changes
N/A
Tests
Charm unit tests.