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

Fix messages getting lost. #191

Closed
monty241 opened this issue Mar 9, 2022 · 5 comments
Closed

Fix messages getting lost. #191

monty241 opened this issue Mar 9, 2022 · 5 comments
Labels
bug This issue is a bug. closed-for-staleness module/logging response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@monty241
Copy link
Contributor

monty241 commented Mar 9, 2022

The logic of AddSingleMessage is unclear from the logic itself. In the previous version, the contents of the message would be replaced always be an error when the queue size is exceeded and the new text was being logged once every five minutes. In all other cases, no message was logged. In all cases, the original cases would be lost without displaying the amount lost.

The reason why messages are not being processed remains to be studied. It occurs despite sufficient sizing and several times a day in a large SAAS-environment.

Our local branch is too much off for easy patching, but the changes in AWSLoggerCore.cs are:

Header

        private static readonly DateTime MIN_TIMESTAMP_UTC_KIND = new DateTime(1, 1, 1, 0, 0, 0, DateTimeKind.Utc);

        /// <summary>
        /// Last time an error message was registered due to overflowing in-memory buffer.
        /// </summary>
        private DateTime _maxBufferTimeStamp = MIN_TIMESTAMP_UTC_KIND;

        /// <summary>
        /// Minimum interval in minutes between two error messages on in-memory buffer overflow.
        /// </summary>
        const double MAX_BUFFER_TIMEDIFF = 5;

AddSingleMessage

The deviating timestamp parameter can be removed, we use it to transport a deviating timestamp since the original event may have occurred minutes earlier due to internal queing of log messages.

        private void AddSingleMessage(string message, DateTime? timestamp)
        {
            DateTime timestampNn = timestamp ?? DateTime.Now;

            if (_pendingMessageQueue.Count > _config.MaxQueuedMessages)
            {
                if (_maxBufferTimeStamp.AddMinutes(MAX_BUFFER_TIMEDIFF) < DateTime.UtcNow)
                {
                    if (_maxBufferTimeStamp == MIN_TIMESTAMP_UTC_KIND)
                    {
                        string errorMessage = $"The AWS Logger in-memory buffer has reached maximum capacity of {_config.MaxQueuedMessages:N0} entries. Currently contains {_pendingMessageQueue.Count:N0} entries.";
                        LogLibraryServiceError(new System.InvalidOperationException(message));
                        _pendingMessageQueue.Enqueue(new InputLogEvent
                        {
                            Timestamp = timestampNn,
                            Message = errorMessage
                        });
                    }
                    _maxBufferTimeStamp = DateTime.UtcNow;
                }
            }

            _pendingMessageQueue.Enqueue(new InputLogEvent
            {
                Timestamp = timestampNn,
                Message = message,
            });
        }
@monty241 monty241 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 9, 2022
@ashishdhingra
Copy link
Contributor

Hi @monty241,

Good afternoon.

Could you please elaborate on the issue and what needs to be investigated, along with supporting code and detailed steps for reproduction?

Thanks,
Ashish

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2022
@github-actions
Copy link

This issue has not received a response in 5 days. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Mar 31, 2022
@monty241
Copy link
Contributor Author

At least the call to _pendingMessageQueue.Enqueue is skipped when the queue is full. The rest remains unclear for reproducibility, but we have seen no more lost message since incorporating it into our fork.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Apr 1, 2022
@ashishdhingra
Copy link
Contributor

At least the call to _pendingMessageQueue.Enqueue is skipped when the queue is full. The rest remains unclear for reproducibility, but we have seen no more lost message since incorporating it into our fork.

@monty241 Unfortunately, we would need consistent reproduction steps to investigate the issue further. And per your response, the issue is no longer reproducible at your end at this stage.

@ashishdhingra ashishdhingra added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 1, 2022
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

This issue has not received a response in 5 days. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness module/logging response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants