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

take the hostname from /etc/host_hostname if the file is there. This … #329

Merged
merged 4 commits into from Oct 2, 2017
Merged

Conversation

ghost
Copy link

@ghost ghost commented Sep 26, 2017

using environment variable SYSLOG_HOSTNAME does not help when logspout is used as a a global service
this changes consists in looking for the file /etc/host_hostname to find the SYSLOG_HOSTNAME
typically, one will run the container with -v/etc/hostname:/etc/host_hostname

When using a compose file, the file will look like something like this

version: "3"

networks:
logging:

services:
logspout:
image: svt2-dtr.am2.cloudra.local/team/logspout:latest
environment:
- ALLOW_TTY=1
networks:
- logging
volumes:
- /etc/hostname:/etc/host_hostname:ro
- /var/run/docker.sock:/var/run/docker.sock
command:
syslog://svt2-logger.am2.cloudra.local:514
deploy:
mode: global
resources:
limits:
cpus: '0.20'
memory: 256M
reservations:
cpus: '0.10'
memory: 128M

…replaces env SYSLOG_HOSTNAME when logspout is run as a global service
@michaelshobbs
Copy link
Member

closes #328

Copy link
Member

@michaelshobbs michaelshobbs left a comment

Choose a reason for hiding this comment

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

One comment inline. Needs docs as well

pid := getopt("SYSLOG_PID", "{{.Container.State.Pid}}")

b, err := ioutil.ReadFile("/etc/host_hostname") // just pass the file name
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Should also check for empty b here.

@michaelshobbs
Copy link
Member

@chris7444 to clarify here, "global service" means a docker service running in a swarm cluster, correct?

@ghost
Copy link
Author

ghost commented Sep 27, 2017

@michaelshobbs global service means there will be one instance of the service (called a task) running on each and every docker node in the swarm. Which is a perfect fit for monitoring agents or containers like logspout. The moment you add a node in the swarm, docker will make sure one replica of the service will run on the new node. The other deployment option is "replicated" where you say I want 'N' instances of the service running in the swarm (eg 3 nginx instances). The swarm makes sure there will always be at least 3 instances of nginx running

Copy link
Member

@michaelshobbs michaelshobbs left a comment

Choose a reason for hiding this comment

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

one docs nitpick but otherwise looks good. have you run this branch in your env?

README.md Outdated
@@ -156,6 +156,44 @@ Logspout relies on the Docker API to retrieve container logs. A failure in the A
* `SYSLOG_TAG` - datum for tag field (default `{{.ContainerName}}+route.Options["append_tag"]`)
* `SYSLOG_TIMESTAMP` - datum for timestamp field (default `{{.Timestamp}}`)

#### Using Logspout in a swarm

In a swarm, logspout is best deployed as a global service. When running logspout with 'docker run', you can change the value of the hostname field using the SYSLOG_HOSTNAME environment variable as explained above. However, this does not work in a compose file because the value for SYSLOG_HOSTNAME will be the same for all logspout "tasks", regardless of the docker host on which they run. To support this mode of deployment, the syslog adapter will look for the file /etc/host_hostname and, if the file exists and it is not empty, will configure the hostname field with the content of this file. You can then use a volume mount to map a file on the docker hosts with the file /etc/host_hostname in the container. The sample compose file below illustrates how this can be done
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: place SYSLOG_HOSTNAME and /etc/host_hostname in backticks

@ghost
Copy link
Author

ghost commented Sep 28, 2017

@michaelshobbs . I did the changes in README.md and yes I did test the changes in my env (pulling the image from my repo)

@ghost
Copy link
Author

ghost commented Oct 2, 2017

@michaelshobbs . What is your release schedule. when can I expect to see the changes merged into your code ? Is there anything I need to do for this to happen ?

@michaelshobbs michaelshobbs merged commit 95b284a into gliderlabs:master Oct 2, 2017
@michaelshobbs
Copy link
Member

@chris7444 just haven't had time to circle back. i'll try to cut a release today. otherwise this is now in master. give it about 30 minutes to show up on docker hub though

@nvanheuverzwijn
Copy link
Contributor

@michaelshobbs @chris7444 Using this feature breaks my logs. I use a service called "logdna" to aggregate all logs. When I mount /etc/host_hostname, the host is correctly displayed but the message is truncated after.

When I hardcode the hostname using SYSLOG_HOSTNAME, everything works fine.

Could it be possible that reading a file with go would add some hidden character ?

@josegonzalez
Copy link
Member

Probably you have a newline character, and we should strip whitespace from the file after its read.

@nvanheuverzwijn
Copy link
Contributor

@josegonzalez There is no newline, I really don't get it. There is no weird character or anything.

@nvanheuverzwijn
Copy link
Contributor

Any way to dump the raw content that is sent to the server ? Some kind of debug mode or something ?

@josegonzalez
Copy link
Member

josegonzalez commented Oct 12, 2017

You're not understanding me. Your file has two lines, and we're reading the whole thing. So the output includes a newline, which is probably causing logs being shipped to be truncated.

The fix is literally for something in our golang code that reads the hostname to trim all whitespace from both ends. If you want to contribute that, that would be great.

@nvanheuverzwijn
Copy link
Contributor

Oh, you are right, I got confused because vim does not show all character :-((

I just checked my file with cat /etc/hostname | od -c and there was effectively a \n character at the end.

billimek added a commit to billimek/logspout that referenced this pull request Nov 1, 2017
* upstream/master: (73 commits)
  better log preface
  null
  toJSON on raw
  fix working_directory so we don't duplicate test runs
  pass debug to test container
  use alpine 3.6 to get golang 1.8.3
  break out hostname setting into func and add simple test
  add test for host_hostname file
  strip \r and \n when reading the file /etc/host_hostname
  minor changes to README.md
  update README.md for swarm deployments PR gliderlabs#329
  address comments PR gliderlabs#329
  take the hostname from /etc/host_hostname if the file is there. This replaces env SYSLOG_HOSTNAME when logspout is run as a global service
  set version to dev on master
  set version to 3.2.3 for release
  update changelog
  fix new golint lintballs
  prepare 3.2.3 release
  Add TAIL environment variable to list of envvars
  review comment - single call to get tail envvar
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants