sematext / sematext-agent-docker

Sematext Docker Agent - host + container metrics, logs & event collector
https://sematext.com/docker
Apache License 2.0
209 stars 30 forks source link

LOGSENE_ENABLED doesn't get recognized as ENV-var #21

Closed szakasz closed 7 years ago

szakasz commented 7 years ago

Hi Stefan,

We have the problem, that when we set LOGSENE_ENABLED as an environment variable then apparently for some reason, it doesn't get recognized. Here's an example output from 'docker inspect' where this was the case:

[
{
    "Id": "55da0101e9e7f77175fb374012621847ad77f4b584376350b5d5100bcfb27f49",
    "Created": "2016-12-12T07:54:20.452304595Z",
    "Path": "/bin/sh",
    "Args": [
        "-c",
        "java -javaagent:/opt/newrelic/newrelic.jar -Dnewrelic.config.license_key=${NEWRELIC_KEY} -Dnewrelic.config.app_name=${NEWRELIC_APPNAME} -Dnewrelic.config.proxy_host=webproxy.sbb.ch -Dnewrelic.config.proxy_port=8080 -jar /deploy/api-gateway.jar"
    ],
...
    "Config": {
        "Hostname": "api-gateway-15-ui8us",
        "Domainname": "",
        "User": "1001100000",
        "AttachStdin": false,
        "AttachStdout": false,
        "AttachStderr": false,
        "ExposedPorts": {
            "8080/tcp": {}
        },
        "Tty": false,
        "OpenStdin": false,
        "StdinOnce": false,
        "Env": [
            "ELAZ_CONFIG_LABEL=integration",
            "ELAZ_CONFIG_SERVER=http://elaz-config-server:8888",
            "ELAZ_PROFILES_ACTIVE=int",
            "JAVA_TOOL_OPTIONS=-Xms512m -Xmx3072m",
            "LOGSENE_ENABLED=false",
            "NEWRELIC_APPNAME=int-api-gateway",
...

In case of Labels, this Feature functions correctly, however the quotation marks are set differently - might that be the root cause?

...
        "Labels": {
            "LOGSENE_ENABLED": "false",
            "Name": "rhel7/rhel",
            "Vendor": "Red Hat, Inc.",
            "Version": "7.2",
            "io.kubernetes.container.hash": "655c979",
            "io.kubernetes.container.name": "akka-singleton",
...
megastef commented 7 years ago

@szakasz thank you for reporting the issue, please try sematext/sematext-agent-docker:1.31.4 (latest)

szakasz commented 7 years ago

@megastef Thanks for the quick fix. I am not sure, but info.LOGSENE_ENABLED === null shouldn't be there on line 107. That will be the case even when LOGSENE_ENABLED is unspecified, for that previously the default of ENABLED was assumed - and that I find correct. I just have made a test with the new sematext-agent on one single node and ALL of our containers have been set to Logsene disabled, even when most of them didn't specify this value. We would like to have further LOGSENE_ENABLED=true in case nothing is specified (at least in the short term, because we cannot ask about 15-20 projects to change their settings in a short time). (Another option might be to have a global setting saying what is the default.)

megastef commented 7 years ago

Sorry, new builds running for 1.30.5 setting LOGSENE_ENABLED to true by default ... https://hub.docker.com/r/sematext/sematext-agent-docker/builds/

szakasz commented 7 years ago

Thanks, now it seems to be ok.

szakasz commented 7 years ago

Hi @megastef I am back regarding LOGSENE_ENABLED. It functions now apparently correct, now we would like to have a minor enhancement (if you want, we can move this to a new issue). We're currently having more than 1000 docker containers (microservices) running on our cluster, and new projects appear almost weekly on our self-service platform. Now due to various reasons (mostly projects forgetting to set it to false explicitly) it's not desirable anymore that LOGSENE_ENABLED=true as default considered is, we would like to have the projects to have set this env.var or label explicitly to true if they want the agent to catch up their logs. Would it be possible to implement thus the following enhancement (or something similar):

In the patterns.yml or as a global ENV VAR there should be a global setting, like LOGSENE_ENABLED_DEFAULT with a default value of true, so that the current environments function the same way, when not doing anything. However if this global variable gets the explicit value of false, then all containers without explicitly set LOGSENE_ENABLED label or env.var will be considered to have this value set to false and thus the agent will not catch up their logs. In any other case or constellation the behaviour would stay like it is now. Thanks in advance for a reply or a solution. :)

megastef commented 7 years ago

+1 for new setting LOGSENE_ENABLED_DEFAULT=false

megastef commented 7 years ago

@szakasz please give it a try:

# get latest dev image
docker pull sematext/sematext-agent-docker:dev

# start SDA, disable container logging for containers 
# having no LOGSENE_ENABLED label or env variable set
docker run -v /var/run/docker.sock:/var/run/docker.sock \
-e LOGSENE_TOKEN=yourLogseneToken \
-e LOGSENE_ENABLED_DEFAULT=false \
-d --name sematext-agent  sematext/sematext-agent-docker:dev

# Run Redis and enable logging for it
docker run -d --name redisWithLogseneEnabled -e LOGSENE_TOKEN=yourLogseneTokenForRedis -e LOGSENE_ENABLED=true redis

# Run Redis - logs should not be collected, because SDA got the flag LOGSENE_ENABLED_DEFAULT=false
docker run -d --name redisWithoutLogsene  redis

# check SDA console to see which containers got logging enabled
docker ps | grep redis
docker logs -f sematext-agent

After tests and positive feedback, we will create a new release. Note: If you don't set LOGSENE_ENABLED_DEFAULT=false SDA sets LOGSENE_ENABLED_DEFAULT=true in the start script to keep the original functionality - so all container logs are collected by default (as expected by most of our users ...).

szakasz commented 7 years ago

Wow, again a fantastic response and resolution time. Thanks a lot, @megastef ! On our side it will take probably a few more days to try this out, because we're just in the midst of a platform upgrade, but we will report back early next week at latest about our tests.

szakasz commented 7 years ago

@megastef We had finally the opportunity to test LOGSENE_ENABLED_DEFAULT=false and it functions perfectly, thanks.

szakasz commented 7 years ago

@megastef Could you please post here a short update when the new release is out? We would like to get this in production in about a week or so. Thanks and have a nice weekend.

megastef commented 7 years ago

This change is already in v1.31.23 (latest). We will check the other issue #26 with the unicode null characters and might release a fix next week.

szakasz commented 7 years ago

Thanks, we are now rolling this change out. After rollout, I have noticed similar SDA output lines: SDA log stats: count=4481 shipped=11 bulks_req_send=4 bulk_req_failed=0 bulk_req_retransmit=0

AFAIK, that means that sematext-agent still gets all the logs from all the Docker containers, but sends only the relevant ones to Logsene. We also have even more extreme cases, where the count is several hundred thousand in a minute and only a tiny fraction of this gets shipped to Logsene. As we are experiencing some Docker performance issues, would it be possible, not to query the Docker API for the LOGSENE_ENABLED=false containers? That could probably save us a lot of Docker API calls and thus precious performance. Shall I make a separate issue for that?

megastef commented 7 years ago

@szakasz Let's discuss this new issue here https://github.com/sematext/sematext-agent-docker/issues/28

megastef commented 7 years ago

@szakasz I assume we can close this issue, right? And follow up on the API call issue #28 ...

szakasz commented 7 years ago

@megastef Sure and thanks a lot. 👍