Skip to content

Conversation

@vilyapilya
Copy link
Contributor

@vilyapilya vilyapilya commented Jan 22, 2020

I was assigning @flush_scheduled to true and did not call the scheduler if the flag was true. I removed this condition. And I also did not call lambda.

Jhonny noticed that he did not see the logs if he used the lib without options. The reason is that, he defines a small number for his buffer limit in the options and the logs got sent because the condition on the line 65 was satisfied. However, when he tried to log without passing the options, this condition was not satisfies because the default buffer size limit is larger than the string he attempted to send.

I tested it by initializing the logger without options
and setting options like this:
options = {
:hostname => 'rubyTestHost',
:ip => '75.10.4.81',
:mac => '00:00:00:a1:2b:cc',
:app => 'rubyApplication',
:level => "INFO",
:env => "PRODUCTION",
:endpoint => "https://logs.logdna.com/logs/ingest",
:flush_interval => 10,
:flush_size => 50000,
:retry_timeout => 60
}

$logger = Logdna::Ruby.new("73523d0836905067b4940f4a718940f9", options)

@vilyapilya vilyapilya changed the title Fix the bug. Remove the condition for scheduling the flush and call lambda. Jan 22, 2020
@vilyapilya vilyapilya requested a review from dchai76 January 22, 2020 01:19
Copy link
Contributor

@dchai76 dchai76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still appreciate a little more detail on the PR description:

  • Why did what we were doing before cause an error (and what was the error)
  • How exactly does this fix it
  • How we verified this works now (i.e. what was the Test Plan)


flush if @flush_limit <= @buffer_byte_size
schedule_flush unless @flush_scheduled
schedule_flush
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've lost context - I'm confused why we need this line (66) when we have line 65. Can you explain?

Copy link
Contributor Author

@vilyapilya vilyapilya Jan 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two reasons why logs are flushed: either the buffer is full or the scheduled time is up. We can configure the library in such a way that it flushes, let's say, every 15 minutes or when there are more than x amount of data in the buffer. If the size limit is 1000 byte, but we sent only 5 bytes, then it will wait for 15 minutes (or whatever time) to send the log.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then should there be some sort of else here? I'm confused why you would ever flush in line 65 and then immediate schedule another flush in line 66.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are right.

@vilyapilya vilyapilya merged commit 1a1c938 into master Jan 23, 2020
@jakedipity jakedipity deleted the fixBug branch January 25, 2021 22:07
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.

4 participants