external-secrets / kubernetes-external-secrets

Integrate external secret management systems with Kubernetes
MIT License
2.6k stars 404 forks source link

Log base w/ configurable pid and hostname keys #868

Closed SocalNick closed 2 years ago

SocalNick commented 2 years ago

hostname isn't always desireable and is somewhat inaccurate given os.hostname is the pod name in kubernetes

SocalNick commented 2 years ago

👋 folks! My company is using k8s-external-secrets...thanks so much for developing / maintaining it!

The pino default log base uses hostname as the key for os.hostname(). In a kubernetes context, this evaluates to the pod name. When our log aggregation platform processes these logs w/ hostname set to the pod name, it can't decorate the logs w/ additional tags for the actual underlying host. This change maintains the default behavior but allows an operator to choose alternate keys for the pid and hostname of the pino log base.

I didn't see any tests around configuration, but if this feels like it requires a test, happy to add one if you can point me in the right direction.

Also, this likely needs to be documented. Can you point me towards the docs that I should update?

Thanks!

Nick

SocalNick commented 2 years ago

@Flydiverny @riccardomc any chance you could take a look at this PR? Thanks!

Flydiverny commented 2 years ago

Regarding docs it would be here: https://github.com/external-secrets/kubernetes-external-secrets/blob/master/charts/kubernetes-external-secrets/README.md#configuration

SocalNick commented 2 years ago

@Flydiverny awesome, thanks for the context on KES vs ESO. I'll take this back to the team and advocate for us making the migration.

As this will take considerable time to migrate, if I make the doc change, are you willing to merge this in?

SocalNick commented 2 years ago

Added documentation, thanks again for your consideration!

Flydiverny commented 2 years ago

Released as 8.4.0