-
Notifications
You must be signed in to change notification settings - Fork 182
PMM-14375: Propagate environment variables to agents #4717
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
base: v3
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3 #4717 +/- ##
==========================================
- Coverage 45.75% 45.72% -0.04%
==========================================
Files 364 364
Lines 37755 37805 +50
==========================================
+ Hits 17275 17286 +11
- Misses 18823 18857 +34
- Partials 1657 1662 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for environment variables to be passed to MongoDB agents (MongoDB Exporter, QAN MongoDB Profiler Agent, and QAN MongoDB Mongolog Agent). The changes enable users to configure custom environment variables for these agents through the API, command-line tools, and internal configuration.
Key changes:
- Added
EnvironmentVariablesfield to theAgentdatabase model with encryption support - Extended API definitions to accept
agent_environment_variablesparameter for MongoDB agents - Updated service creation and configuration logic to pass environment variables to agents
- Added database schema migration to support the new field
- Included command-line support for setting environment variables via the admin tool
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| managed/models/agent_model.go | Adds EnvironmentVariables field to Agent struct with getter/setter methods |
| managed/models/database.go | Adds database migration to create environment_variables column |
| managed/models/encryption_helpers.go | Adds encryption support for environment variables |
| managed/services/management/mongodb.go | Passes environment variables when creating MongoDB agents |
| managed/services/inventory/agents.go | Retrieves and passes environment variables for MongoDB agents |
| managed/services/agents/mongodb.go | Constructs environment variables for MongoDB exporter and QAN agents |
| api/inventory/v1/agents.proto | Adds agent_environment_variables field to MongoDB agent API definitions |
| api/management/v1/mongodb.proto | Adds agent_environment_variables field to MongoDB service params |
| admin/commands/management/add_mongodb.go | Adds CLI support for agent environment variables |
| api-tests/management/mongodb_test.go | Fixes variable naming typo in test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BupycHuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this env variable is passed to QAN agent.
| } | ||
|
|
||
| // GetEnvironmentVariables decodes environment variables. | ||
| func (s *Agent) GetEnvironmentVariables() (map[string]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if error isn't used anywhere why do we return it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was because of the underlying getLabel() function - will update the callers to use the errors.
Based on the discussion here, I don't think we need to do anything special to share env variables with qan agents - they are still part of the original pmm-agent process and have access to the same env variables - will remove them from the proto definitions for qan agents as well |
yeahh opened to a different flow really, one option will be to automatically pass pmm-agent environment variables to exporters. Sharing all of it might not be a good idea, hence, we might still need some flag to specify what variables should be shared (only difference is that this time, user only specifies the variable names, not values). |
PMM-14375
Link to the Feature Build: SUBMODULES-0