Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Firehose topic for IncidentLogger #96

Merged
merged 0 commits into from

3 participants

@grantr

IncidentLogger currently doesn't support the perfectly valid logging need to have every message logged instead of just errors.

This branch adds a firehose topic that gets every LogEvent above the configured log level. An example FirehoseConsumer is also included that just wraps a ::Logger instance.

This feature has the potential to cause serious slowdowns if the fanout notifier is not performant. I haven't done any benchmarks yet but someone probably should before the IncidentLogger is enabled by default.

Speaking of things that should happen before IncidentLogger is enabled: still no tests.

Also something that should happen before this goes out is a shutdown hierarchy. Occasionally the notifier gets shut down before the consumers. This is a bigger problem now because the logger tries to publish the shutdown message, but gets a DeadActorError which is logged to STDERR by the fallback logger.

@tarcieri
Owner

Sorry I haven't had time to look at this yet, been pretty busy with Strange Loop

@grantr

No problem, good luck at the conference.

@grantr grantr merged commit f07e038 into from
@tarcieri
Owner

This doesn't have non-negligible perf impacts, right?

@grantr

I don't really know, I haven't tested perf yet. It's mostly dependent on performance of the fanout notifier.

@grantr

oh snap this was not supposed to be merged into master.. when did i get commit rights!?!

@tarcieri
Owner

You've had them for awhile ;) Feel free to revert this if you don't think it's ready

@grantr

Seems like this was caused by me rebasing the branch to master and accidentally removing the commit. That seems to have closed the PR even though nothing was merged.

I'll put this branch back in place and work somewhere else.

@halorgium
Owner
@tarcieri
Owner

@halorgium I was confused because it claims it was merged even though it was not

@grantr

Ok I pushed the old branch again, but I can't figure out how to re-open this. I'll create a new PR.

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

Sorry, commit information is not available for this pull request.

Something went wrong with that request. Please try again.