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 environment variables to the system process metricset #3337

Merged
merged 2 commits into from Jan 12, 2017

Conversation

Projects
None yet
3 participants
@andrewkroh
Copy link
Member

commented Jan 11, 2017

This PR adds the environment variables that were used to start the process to the data reported in the system process metricset. The data is added as a dictionary under the system.process.env key. Environment variables must be whitelisted using an array of regular expressions specified using process.env.whitelist: [] in the module config.

This feature implemented for FreeBSD, Linux, and OS X.

Sample config:

metricbeat.modules:
- module: system
  metricsets: [process]
  process.env.whitelist: ['USER', 'PATH']
@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2017

What kind of configuration options if any (like filtering, whitelist, blacklist, enabled) should we provide with this feature?

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Jan 11, 2017

I would only add process.env.enabled: true as a config option. Filtering can be done with the processors if needed. I think with the processors we should also be able to only have the variables for certain processes and remove it in all other cases etc.

I'm unsure if we should enabled it by default. On the one hand the info can be quite useful, but it can also contain quite a list of defaults which are the same for all processes.

@tsg

This comment has been minimized.

Copy link
Collaborator

commented Jan 11, 2017

@joegallo @alexbrasetvik FYI, we're going this direction for Metricbeat.

@tsg

This comment has been minimized.

Copy link
Collaborator

commented Jan 11, 2017

I'd say we should have a whitelist and the feature should be disabled by default due to privacy concerns. Secrets are often stored in env, so we have to make it hard to leak them accidentally.

@tsg

This comment has been minimized.

Copy link
Collaborator

commented Jan 11, 2017

To be clear, I mean a whitelist on the variable names, so something like process.env.variables: ["PATH"].

@andrewkroh andrewkroh force-pushed the andrewkroh:feature/process-env-vars branch from 836d24d to bd054f9 Jan 11, 2017

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2017

I added process.env.whitelist: [] to control what environment variables are added to the system process events. It accepts a list of regular expressions. When empty or unset, no environment variables are added to the process event.

This should be updated to the the ExactMatcher from #2433 when it merges.

This PR depends on elastic/gosigar#62 and needs to be updated after that merges to put the commit hash into the glide.yaml. (This PR has been updated.)

@ruflin

ruflin approved these changes Jan 12, 2017

Copy link
Collaborator

left a comment

andrewkroh added some commits Jan 10, 2017

Update gosigar version to b49e01eb1e5c68c469392a63feaccae9352ceb12
This gets the ProcArgs implementation.
Add environment variables to the system process metricset
This PR adds the environment variables that were used to start the process to the data reported in the system process metricset. The data is added as a dictionary under the `system.process.env` key. Environment variables must be whitelisted using an array of regular expressions specified using `process.env.whitelist: []` in the module config.

This feature implemented for FreeBSD, Linux, and OS X.

@andrewkroh andrewkroh force-pushed the andrewkroh:feature/process-env-vars branch from bd054f9 to 6e23846 Jan 12, 2017

@andrewkroh andrewkroh added review and removed in progress labels Jan 12, 2017

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2017

The PR is now updated with the final gosigar commit hash.

@ruflin ruflin merged commit 82b28c6 into elastic:master Jan 12, 2017

3 of 4 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
CLA Commit author has signed the CLA
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Build finished.
Details

andrewkroh added a commit to andrewkroh/beats that referenced this pull request Feb 28, 2017

Document process.env.whitelist config option
Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for elastic#3337.

ruflin added a commit that referenced this pull request Mar 3, 2017

Document process.env.whitelist config option (#3694)
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for #3337.

* Add reason behind default behavior for process env vars

monicasarbu added a commit to monicasarbu/beats that referenced this pull request Mar 4, 2017

Document process.env.whitelist config option (elastic#3694)
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for elastic#3337.

* Add reason behind default behavior for process env vars

(cherry picked from commit f9ac9f2)

ruflin added a commit that referenced this pull request Mar 6, 2017

Document process.env.whitelist config option (#3694) (#3722)
* Document process.env.whitelist config option

Document `process.env.whitelist` which is used by the Metricbeat system process metricset to specify what environment variables should be captured.

Adds documentation for #3337.

* Add reason behind default behavior for process env vars

(cherry picked from commit f9ac9f2)

@monicasarbu monicasarbu deleted the andrewkroh:feature/process-env-vars branch Mar 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.