caddyserver / transform-encoder

Log encoder module for custom log formats
Apache License 2.0
79 stars 17 forks source link

Return `-` when value is not present #1

Closed danlsgiga closed 3 years ago

danlsgiga commented 4 years ago

I'm using the format-encoder module in Caddy v2 to format my logs with a few headers and other fields. I noticed that for example, if I specify the template as:

'{common_log} {request>headers>Referer} {request>headers>User-Agent} {request>host} {resp_headers>X-Request-Id} {duration}'

Sometimes HTTP requests won't have the Referer request header (first access to the page). In those situations, its cleaner (IMO) to return - instead of the placeholder.

I've got logs being generated like this:

127.0.0.1 - - [06/Oct/2020:22:00:23 +0000] "GET /.well-known HTTP/2.0" 302 0 {request>headers>Referer} ["Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:81.0) Gecko/20100101 Firefox/81.0"] localhost ["1d3c101945cc400196a1d5518e7a212f"] 0.00328956

Same thing when the {request>headers>User-Agent} is not present:

127.0.0.1 - - [06/Oct/2020:22:00:23 +0000] "GET /.well-known HTTP/2.0" 302 0 {request>headers>Referer} {request>headers>User-Agent} localhost ["76059f3e7b374da2a4e24a80578c3b29"] 0.00267243

mohammed90 commented 4 years ago

I think the fix should be as simple as replacing ReplaceKnown(se.Template, "") with ReplaceAll(se.Template, "-"):

https://github.com/caddyserver/format-encoder/blob/addf3285dafcd73d9a5f477a236049ec31a4ce2d/formatencoder.go#L100

I'll get to it and confirm this coming weekend, if that's fine and not too urgent.

mholt commented 4 years ago

How do we know that's what all users want though? Common Log Format uses - as the "empty" placeholder, but how do we know that a user's template is designed to extend CLF?

mohammed90 commented 4 years ago

Make an additional config knob?

danlsgiga commented 4 years ago

I'll get to it and confirm this coming weekend, if that's fine and not too urgent.

no rush and thanks!

mholt commented 4 years ago

Make an additional config knob?

Possibly. We'd need to discuss a design proposal though, simple as it might be. I am not immediately sure how it will look.

mohammed90 commented 4 years ago

Sorry, so I didn't finish this last weekend, but playing checking out the various options to take. The current Caddyfile syntax is:

format formatted [<template>]

The two options I can think of are: 1-

format formatted [<template> [<placeholder>]]

2-

format formatted [<template>] { # the block is optional with '-' being the default placeholder when empty
    placeholder[<placeholder>]
}

Thoughts? Preferences?

danlsgiga commented 4 years ago

I'd prefer the option 2. Set sane default (-) but give the flexibility to customize it.