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 ability to exclude containers similar to filter includes #258

Open
rocketraman opened this issue Jan 29, 2017 · 17 comments
Open

Add ability to exclude containers similar to filter includes #258

rocketraman opened this issue Jan 29, 2017 · 17 comments

Comments

@rocketraman
Copy link

We can use filters to include specific containers by id, name, etc. It would be useful to exclude containers in the same way.

Sometimes the environment / label approach does not work since the container is managed by a third party e.g. on a cloud environment, the hosting provider may install and manage containers that provide services specific to that environment. We don't have control over these containers, so it may not be convenient or possible to set their environment or labels.

@dcassiero
Copy link

This would be useful. We use Rancher and recently upgraded to 1.2.x which introduced new infrastructure containers. One of which is extremely chatty. There is no way to turn off logging for these containers or add LOGSPOUT=ignore to their run command. The routesapi is not useful here. We do Rancher+logspout->Loggly.

@josegonzalez
Copy link
Member

Would this pr be useful for that? https://github.com/gliderlabs/logspout/pull/222/files

@rocketraman
Copy link
Author

@josegonzalez Well, it does get us part of the way there -- it lets us explicitly label all the containers that we wan to include. But unfortunately, now we have the opposite problem. If there are infrastructure containers that we do want to include, we have no way of including them.

@rocketraman
Copy link
Author

rocketraman commented Jan 31, 2017

I think this is the situation:

@josegonzalez
Copy link
Member

Mind making a pull request?

@rocketraman
Copy link
Author

rocketraman commented Jan 31, 2017

Mind making a pull request?

I wouldn't, though I've never written any Go code before and for some reason I find the syntax somewhat opaque and unreadable (:=? what was wrong with just =?). Everyone else seems to love it, so it must be just me :-)

But this is an easy enough task to do. I might have time to work on this in about a month or so. If no one has tackled it by then, I'll likely do so. If I implement it, I'd set it up so all the following rules all must be true for a container to be logged:

  1. Container must match all include filters. If no include filter(s) are provided, all containers are included (i.e. equivalent of filter.name="*"). If filters are not currently multi-valued parameters, make them so.

  2. Container must not be a part of any exclude filters. If no exclude filter(s) are provided, no containers are excluded. Exclude filters will have the same syntax as include filters, except they are prefixed with !.

  3. If a logspout INCLUDE_LABEL label was provided, then container must be labeled with the configured include label set to true or 1.

  4. If a logspout EXCLUDE_LABEL label was provided, then container must not be labeled with the configured exclude label set to true or 1.

  5. If an environment variable LOGSPOUT is present, it must not be set to ignore or false or 0.

  6. If an environment variable LOGSPOUT is present, it must be set to true or 1.

Now, you would be able to default to exclude via a filter like filter.name=!* and then explicitly include via environment variable or label. Or you can default to include with no filter, and then explicitly exclude via environment variable or label. Or you can just specify everything in a set of filters. Not sure if the way the current code uses filters would easily support this logic.

@dcassiero
Copy link

An exclude filter would be desirable since I know for a fact how many, and what names the exclude containers will be. Is that something that is easier to implement? The dozens of other arbitrary containers that we know need to be logged and I will never know how many of them there are or what their name might be (--name).

@garceri
Copy link

garceri commented Feb 9, 2017

@dcassiero : You can add the LOGSPOUT=ignore env variable to the infra stacks by "Upgrading" them, Select the Service within the stack you want to apply the change, click on the 3 dots, then select Upgrade, you will be presented with a screen where you can add the environmental variable, this is how i did it.
Unfortunately this solved only part of the issue as we are still experiencing very high cpu usage on dockerd after starting logspout with the logstash adapter as described here

@michaelshobbs
Copy link
Member

@dcassiero LOGSPOUT=ignore seems like it would work for this case. Closing for now. Please comment if this is not sufficient for your use case.

@rocketraman
Copy link
Author

@michaelshobbs @dcassiero was not the original reporter. Not sure why your comment addresses his use case. Unless there is a solution for my use case, not sure why this was closed.

@michaelshobbs
Copy link
Member

Apologies. Re-opening

@michaelshobbs
Copy link
Member

Not sure I have the cycles to spend on this enhancement but I'd definitely help guide a PR through the process.

@rocketraman
Copy link
Author

@michaelshobbs Not sure when I'd get around to it, but what did you think of my proposed approach outlined above? #258 (comment)

@michaelshobbs
Copy link
Member

michaelshobbs commented Jun 7, 2017

:=? what was wrong with just =?

both are used in golang. declaration and assignment vs assignment 😄

However, to your approach. Generally speaking, I'm pretty averse to complex rules like you outlined. That being said, if you can simplify the filtering functionality and/or make it crystal clear in documentation, that would be preferable. Also please include test cases to validate these rules and finally, thank you if you do end up getting a PR put together.

@rocketraman
Copy link
Author

Generally speaking, I'm pretty averse to complex rules like you outlined.

Agreed. The primary complexity comes from trying to maintain backward compatibility while also adding function. If you're ok with making changes that are not backward compatible, I suspect it could be drastically simplified.

@michaelshobbs
Copy link
Member

I'm fine with bc-breakage as long as it's clearly outlined in docs and in the PR for the community to review.

@rocketraman
Copy link
Author

@michaelshobbs Here is a proposed simplification. Everything is controlled by filters, but filters can refer to labels and environment variables, and be negated. In detail:

For a container to be logged, it must pass the filter, and the filter works like a simplified rsync --include and --exclude. This is very flexible and powerful but relatively simple. To produce the complete filter, we'll use multi-valued URI params include and exclude, with ordering. See below for examples.

Filters may have the form:

*
name:<expression>
label:<labelname>:<expression>
env:<variablename>:<expression>

"expression" is basically a regex, but it may contain references to environment variables. Perhaps a Go template is appropriate here? I'm going to assume a simple expression format for now, and revisit its exact representation at implementation time.

"labelname" may (this is open for discussion) be an environment variable reference -- see the examples below. This is would be necessary for backward compatibility.

Examples

To include all containers by default, but exclude containers having a label "logspout_exclude" with value "true":

?include=*&exclude=label:logspout_exclude:true

To include all containers by default, but exclude containers having an environment variable LOGSPOUT with value "ignore":

?include=*&exclude=env:LOGSPOUT:ignore

To include only containers having a "LOGSPOUT" environment variable with value "true", and exclude all others:

?include=env:LOGSPOUT:true

To include all containers matching a given label, except for ones with name foo and bar:

?include=label:logme:*&exclude=name:foo&exclude=name:bar

If no filters are specified, the default filter would be represented by the following (this should be backward compatible I think):

?include=*&exclude=label:$EXCLUDE_LABEL:*&exclude=env:LOGSPOUT:ignore

NOTE: here, the label-based exclude refers to the environment variable $EXCLUDE_LABEL, so the exclude label is actually definable at runtime via the environment, as today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants