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

LOGGING-3416 Enable verbose on log forwarder #164

Merged
merged 4 commits into from
Oct 14, 2020

Conversation

jsubirat
Copy link
Contributor

@jsubirat jsubirat commented Oct 8, 2020

The purpose of this PR is to be able to enable the verbose mode of Fluent Bit when running the agent in verbose mode. This will greatly help when diagnosing problems in the Log Forwarder.

Also, I took the opportunity to remove the logs/logs folder, which I think was accidentally copied twice and has no use.

@@ -298,6 +298,7 @@ func initializeAgentAndRun(c *config.Config, logFwCfg config.LogForward) error {
FluentBitExePath: c.FluentBitExePath,
FluentBitNRLibPath: c.FluentBitNRLibPath,
FluentBitParsersPath: c.FluentBitParsersPath,
FluentBitVerbose: c.Verbose != 0,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure whether to enable it when Verbose != 0 or just when Verbose = 1. Also, I am afraid that when Verbose = 3, when shipping the infra-agent logs (and those logs containing the logs being shipped, due that is in verbose mode), this might cause an infinite loop. Maybe it would be wiser to just enable it when Verbose = 1? Any thoughts @carlosroman @varas @cristianciutea ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@varas varas Oct 9, 2020

Choose a reason for hiding this comment

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

@jsubirat did you experience a retro feedback loop enabling verbosity on both agent and FluentBit?

if that's the case, we should add a circuit breaker

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested it now and actually doesn't cause an infinite loop, thanks to the fact that we're not outputting the forwarded records. However, a hacky mind could define an external fluent-bit configuration file to be included (via the @INCLUDE tag) with the following contents:

[OUTPUT]
    Name stdout
    Match *

And this does cause lots of stuff being written to the infra-agent log, as the records themselves are sent to the infra-agent log, which in turn are captured by fluent-bit, which in turn sends that to the log. It has an exponential growth (about 250MB/min in my VM).

I'd recommend, then, to only enable the verbose mode of Fluent Bit when running with Verbose = 2, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a change for FB verbose to be enabled as a feature trace

@coveralls
Copy link

coveralls commented Oct 8, 2020

Pull Request Test Coverage Report for Build 306280047

  • 0 of 16 (0.0%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.9%) to 57.501%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/newrelic-infra/newrelic-infra.go 0 1 0.0%
pkg/trace/features.go 0 2 0.0%
pkg/integrations/v4/supervisor.go 0 4 0.0%
pkg/integrations/v4/supervisor_fb.go 0 4 0.0%
pkg/trace/trace.go 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
internal/agent/event_sender_vortex.go 1 73.21%
Totals Coverage Status
Change from base Build 306176212: -0.9%
Covered Lines: 10717
Relevant Lines: 18638

💛 - Coveralls

Copy link
Contributor

@varas varas left a comment

Choose a reason for hiding this comment

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

I'm wondering whether we should forward FB verbosity as:

  • log.Debug entries
  • or as a feature trace

Waiting for team output on this one.

In the later users would need to enable verbose + the feature trace to get FB verbosity logged.
Config for the later would be like trace: [ log-forwarder ].
This is the same approach we use for other features to avoid flooding the agent output.
Helpers per feature are available at https://github.com/newrelic/infrastructure-agent/blob/master/pkg/trace/features.go#L13-L21

@varas varas force-pushed the LOGGING-3416_enable_verbose_on_log_forwarder branch 2 times, most recently from 2afbcd5 to 9a6a70e Compare October 9, 2020 13:33
@varas varas force-pushed the LOGGING-3416_enable_verbose_on_log_forwarder branch from 9a6a70e to 1180fe3 Compare October 9, 2020 13:38
@@ -18,6 +18,7 @@ const (
DM_SUBMISSION Feature = "dm.submission" // dimensional metrics submission
METRIC_MATCHER Feature = "metric.match" // match metric by rule
INVENTORY Feature = "inventory"
LOG_FWD Feature = "log.fw"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is what needs to be written by the user, ie:

trace:
  - log.fw

maybe we should be more verbose on the name? like log.forwarder?

varas
varas previously approved these changes Oct 9, 2020
@carlosroman carlosroman merged commit 48fec4b into master Oct 14, 2020
@carlosroman carlosroman deleted the LOGGING-3416_enable_verbose_on_log_forwarder branch October 14, 2020 10:39
Copy link
Contributor

@carlosroman carlosroman left a comment

Choose a reason for hiding this comment

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

Oops, merged but just wanted add an okay from us.

@varas
Copy link
Contributor

varas commented Oct 14, 2020

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