-
Notifications
You must be signed in to change notification settings - Fork 59
Add synchronous mode for serverless environments #227
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| using System; | ||
| using StatsdClient.Statistic; | ||
|
|
||
| namespace StatsdClient.Bufferize | ||
| { | ||
| /// <summary> | ||
| /// IStatsBufferize defines the contract for sending stats to the pipeline. | ||
| /// </summary> | ||
| internal interface IStatsBufferize : IDisposable | ||
| { | ||
| bool TryDequeueFromPool(out Stats stats); | ||
|
|
||
| void Send(Stats stats); | ||
|
|
||
| void Flush(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| using System; | ||
| using StatsdClient.Statistic; | ||
|
|
||
| namespace StatsdClient.Bufferize | ||
| { | ||
| /// <summary> | ||
| /// SynchronousSender sends metrics synchronously on the calling thread. | ||
| /// This is intended for serverless environments (e.g., AWS Lambda) where | ||
| /// background threads may be frozen between invocations. | ||
| /// </summary> | ||
| internal class SynchronousSender : IStatsBufferize | ||
| { | ||
| [ThreadStatic] | ||
| private static Stats _threadLocalStats; | ||
|
|
||
| private readonly StatsRouter _statsRouter; | ||
| private readonly Action<Exception> _optionalExceptionHandler; | ||
| private readonly object _lock = new object(); | ||
|
|
||
| public SynchronousSender(StatsRouter statsRouter, Action<Exception> optionalExceptionHandler = null) | ||
| { | ||
| _statsRouter = statsRouter ?? throw new ArgumentNullException(nameof(statsRouter)); | ||
| _optionalExceptionHandler = optionalExceptionHandler; | ||
| } | ||
|
|
||
| public bool TryDequeueFromPool(out Stats stats) | ||
| { | ||
| if (_threadLocalStats == null) | ||
| { | ||
| _threadLocalStats = new Stats(); | ||
| } | ||
|
|
||
| stats = _threadLocalStats; | ||
| return true; | ||
| } | ||
|
|
||
| public void Send(Stats stats) | ||
| { | ||
| try | ||
| { | ||
| lock (_lock) | ||
| { | ||
| _statsRouter.Route(stats); | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _optionalExceptionHandler?.Invoke(e); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it is a good idea to swallow the exceptions if a handler isn't specified. I understand the purpose, given asynchronous does hide the exceptions, it does mean you can seamlessly swap between synchronous and asynchronous. But I imagine someone using a synchronous sender would expect the exceptions to be surfaced right away.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it, to keep consistent with the async version, we should have the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated: when |
||
| } | ||
| } | ||
|
|
||
| public void Flush() | ||
| { | ||
| try | ||
| { | ||
| lock (_lock) | ||
| { | ||
| _statsRouter.Flush(); | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _optionalExceptionHandler?.Invoke(e); | ||
| } | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| Flush(); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming of this interface is confusing. It doesn't really capture what it is trying to do. I think
IStatsSenderwould make more sense.Then perhaps
SynchronousSenderandAsynchronousBufferizedSenderimplementing them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed as suggested:
IStatsBufferize→IStatsSender,StatsBufferize→AsynchronousBufferizedSender. Also renamed the factory types for consistency (IStatsBufferizeFactory→IStatsSenderFactory,StatsBufferizeFactory→StatsSenderFactory).