aptible / supercronic

Cron for containers
MIT License
1.84k stars 112 forks source link

Structured logging: facilitate additional fields in log lines #147

Open chriskellyrelay opened 6 months ago

chriskellyrelay commented 6 months ago

We're working on adding more structure to our logging to help us be able to correlate things in a complex environment. To facilitate this, we'd like to have supercronic facilitate adding additional key+value pairs to it's JSON log output.

For our specific use case, we could fork supercronic to do something like:

return logrus.Fields{
  "logger":           "supercronic",
  "environment_name": os.GetEnv("ENVIRONMENT_NAME"),
  "service_name":     os.GetEnv("SERVICE_NAME"),
  "release_version":  os.GetEnv("RELEASE_VERSION"),
}

Maintaining a fork isn't great, so alternatively the ability to pass this in via command lines flags would work. Either "dynamically":

    "command": [
      "/usr/bin/supercronic",
      "-json",
      "-passthrough-logs",
      "-log-metadata-from-env environment_name=ENVIRONMENT_NAME",
      "-log-metadata-from-env service_name=SERVICE_NAME",
      "-log-metadata-from-env release_version=RELEASE_VERSION",
      "./crontab"
    ]

or statically:

    "command": [
      "/usr/bin/supercronic",
      "-json",
      "-passthrough-logs",
      "-log-metadata environment_name=foo",
      "-log-metadata service_name=bar",
      "-log-metadata release_version=baz",
      "./crontab"
    ],

Are either of these approaches preferable? Would you accept a pull requests for it?

UserNotFound commented 6 months ago

The dynamic example looks good! My first thought was whether the option could be provided more tersely, eg, inferring the field name if only an ENV variable is specified?

For example, I would think the most common case would be that the field name and environment variable match --log-metadata ENVIRONMENT_NAME could be specified to log the value of $ENVIRONMENT_NAME to the field name "environment_name"

In the (slightly) more complex case you could specify both the field and variable names, when they differ: --log-megadata release_version=APTIBLE_GIT_REF would log the value of $APTIBLE_GIT_REF to the field name "release_version"

Would you accept a pull requests for it?

If it includes appropriate tests, and doesn't alter any existing use cases, we'd be happy to review such a PR!

chriskellyrelay commented 5 months ago

Sounds good, --log-metadata ENVIRONMENT_NAME works for us and could help a lot of people. Once things are in logs it is "easy enough" to transform them later if needed.

I have a ticket internally for this, will update here if/when we get the bandwidth to work on it.