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

add in worker logging using zerolog and logrus #1170

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

reillyse
Copy link
Contributor

@reillyse reillyse commented Jan 9, 2025

Description

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

What's Changed

Added combined logging to the worker. Now users can bring their own logger and by calling

logger := context.NewCombinedLogrusLogger(logrusLogger)

or

logger  := context.NewCombinedZerologLogger(zerologLogger)

they can log to hatchet and to their local logging.

Copy link

vercel bot commented Jan 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hatchet-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2025 2:52am

@grutt
Copy link
Contributor

grutt commented Jan 9, 2025

hm... i dont think i like this pattern and think what you were going for in the spec makes more sense. can we not expose a register logger or something on the worker or client to set the logger fn globally?

i don't think folks will want to init in every step

@nikoksr
Copy link

nikoksr commented Jan 9, 2025

@grutt I was coincidentally working on something similar. We're actively using and building on Hatchet and need a way to plug in our own logger. Submitted #1171 as a draft to quickly chip in an alternative to this here PR.

@reillyse reillyse requested a review from abelanger5 January 14, 2025 00:13
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