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

Attach context #112

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Attach context #112

wants to merge 5 commits into from

Conversation

joromir
Copy link
Contributor

@joromir joromir commented Jul 3, 2018

  • Improve passing context to logger
  • Refactor code in Loga::Configuration.

Fixes #77

@joromir
Copy link
Contributor Author

joromir commented Jul 6, 2018

👀

@bliof
Copy link
Contributor

bliof commented Jun 19, 2019

🤔 Unfortunately the current implementation won't do / needs expanding.

The main problem is that when we trace something e.g. sidekiq job or a rack request we want to pass some metadata to that trace e.g. Loga.context(user_type: 'Admin')

Overall we can refactor a bit more and extract the tracing so from e.g. Loga::Sidekiq::JobLogger#call and Loga::Rack::Logger#call when the trace starts we can create a context for that trace and with the Loga.context(user_type: 'Admin') we start filling that context.

# somewhere in Loga::Rack::Logger#call

Loga::Trace.trace do
   @app.call(env).tap { |status, _headers, _body| data['status'] = status.to_i }
end

# somewhere in the app

Loga.context(user_type: 'Admin')

# or even we can create nested traces/contexts

Loga.context(user_type: 'Admin') do
   Loga.logger.info "Call 1" # -> Call 1 user_type=Admin
   jobs.each do |job|
     Loga.context(job_id: job.id) do
        Loga.logger.info "Call 2" # -> Call 2 user_type=Admin job_id=123
     end
   end
   Loga.logger.info "Call 3" # -> Call 3 user_type=Admin
end

I'll continue the PR ☝️ direction. The dsl is still questionable ;)

@cassiomarques
Copy link
Contributor

Closing, stale and probably no longer needed.

@bliof
Copy link
Contributor

bliof commented Feb 21, 2020

Needed but not trivial to implements so.. we haven't spend the time :) -> Opening again

@bliof bliof reopened this Feb 21, 2020
@cassiomarques
Copy link
Contributor

ivial to implements so.. we haven't spend the time :) -> Opening again

Would the code here be reused or would the author probably start from scratch?

@bliof
Copy link
Contributor

bliof commented Feb 21, 2020

No idea. If/when I try I'll use the current PR. Assume this PR as an Issue with some code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve passing context to logger
4 participants