Enhancement in plugin configuration and bug fixes #61
Conversation
This file contains 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
* Plugin can be configured from provisioning file and/or env vars * Env vars will always have higher precedence and overwrite existing values from file * New config option theme has been added for report generation Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
* Seems upon finishing API request for report, worker pool is entering into busy loop hogging up CPU. This has been fixed using background context for worker pool and stopping pool once context channel is closed * We dispose the pool in dispose method safely to ensure that there are no leaks * Files have been renamed to `pool.go` Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
* Add doc strings and remove unused files Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
mahendrapaipuri
added
documentation
Seems like that. Good catch! |
jkroepke
approved these changes
Aug 8, 2024
* Overriding chrome entrypoint in e2e TLS tests. Add --ignore-certificate-errors=1 to chromium instance launch command * Remove test case 10.4.3 from matrix. Currently we are not using cookies to make requests in playwright tests. So this case is of no value * Generate pdf diff file and add it to artifacts for better debugging experience * Reports generated by local and remote chrome instances have small pixel offset. So we use separate reports for comparison depending on which type of chrome instance we are using in e2e test Signed-off-by: Mahendra Paipuri <mahendra.paipuri@gmail.com>
mahendrapaipuri
force-pushed
the
config_enhancements
branch
from
a3a29a9
to
63721c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Features:
GF_REPORTER_PLUGIN_*to avoid clashing with env vars from Grafana and other plugins.themeconfig option has been added to control the theme of the panels in the report.Bugfix:
Seems like there is a bug in the worker pool implementation. Upon finishing an API request to
reportendpoint, the context channel is being closed and thus the loop goes into busy loop due tocontinuestatement. This is hogging up the CPU a lot. To reproduce, spin up docker compose developement env, make a API request toreportend point and once the report has been generated, watch the CPU usage of plugin process.Not sure why the context is being closed here. So the fix was to
returnupon the closure of context channel and use background context for the workers. When plugin app instance is being disposed, we cancel the context indisposereceiver safely releasing the resources owned by worker pool.@jkroepke Was it a typo to have
continuestatement there?CI:
e2e test matrix has been updated to add more tests with and without remote chrome and service accounts
Docs:
READMEhas been updatedCloses #56