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 a parameter to docker prospector to filter on stream #6057

Merged
merged 3 commits into from
Jan 16, 2018

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Jan 12, 2018

Sometimes you are only interested on stdout or stderr, this parameters
allows to filter the input and only read one of them:

prospectors:
  - type: docker
    containers.stream: stderr
    containers.ids:
      - '*'

Sometimes you are only interested on stdout or stderr, this parameters
allows to filter the input and only read one of them.
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a CHANGELOG entry?

return &DockerJSON{reader: r}
func NewDockerJSON(r Reader, stream string) *DockerJSON {
return &DockerJSON{
stream: stream,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should validate that it's a valid stream name.

@@ -85,7 +85,7 @@ type config struct {
JSON *reader.JSONConfig `config:"json"`

// Hidden on purpose, used by the docker prospector:
DockerJSON bool `config:"docker-json"`
DockerJSON string `config:"docker-json"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using the internal parameter to enable docker parsing and pass the desired stream through it, possible values:

  • "" - default, docker parsing is disabled
  • all - default for docker prospector, read all streams
  • stdout
  • stderr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, didn't realise you passed through the stream param.

@exekias
Copy link
Contributor Author

exekias commented Jan 16, 2018

Thank you for the review @ruflin, this should be ready for a second look 😄

@ruflin ruflin merged commit b274aca into elastic:master Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants