Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Log base w/ configurable pid and hostname keys #868

Merged

Conversation

SocalNick
Copy link
Contributor

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

- `hostname` isn't always desireable and is somewhat inaccurate given
os.hostname is the pod name in kubernetes
@SocalNick
Copy link
Contributor Author

👋 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
Copy link
Contributor Author

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

Copy link
Member

@Flydiverny Flydiverny left a comment

Choose a reason for hiding this comment

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

Also see #864

@SocalNick
Copy link
Contributor Author

@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
Copy link
Contributor Author

Added documentation, thanks again for your consideration!

@Flydiverny Flydiverny merged commit ca549f5 into external-secrets:master Nov 17, 2021
@Flydiverny
Copy link
Member

Released as 8.4.0

@SocalNick SocalNick deleted the nc/log-base-configurable-keys branch November 17, 2021 22:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants