Skip to content
This repository was archived by the owner on Aug 30, 2019. It is now read-only.

cmd/trace-agent: ignore SIGPIPE signal#486

Merged
gbbr merged 3 commits into
DataDog:masterfrom
remicalixte:remicalixte/fix-journald
Oct 12, 2018
Merged

cmd/trace-agent: ignore SIGPIPE signal#486
gbbr merged 3 commits into
DataDog:masterfrom
remicalixte:remicalixte/fix-journald

Conversation

@remicalixte

@remicalixte remicalixte commented Oct 9, 2018

Copy link
Copy Markdown

What does this PR do?

By default systemd redirect the stdout to journald. When journald is stopped or crashes we receive a SIGPIPE signal. Go ignore SIGPIPE signals unless it is when stdout or stdout is closed, in this case the agent is stopped. We don't want that so we intercept the SIGPIPE signals and just discard them.

Motivation

Fix DataDog/datadog-agent#1555

Related datadog-agent PR: DataDog/datadog-agent#2416

@palazzem
palazzem requested a review from gbbr October 9, 2018 19:21
@palazzem palazzem added this to the 6.6.0 milestone Oct 9, 2018
@gbbr gbbr changed the title Don't stop the agent when journald is stopped or crashes flags: add flag to allow ignoring SIGPIPE signals Oct 10, 2018
@gbbr

gbbr commented Oct 10, 2018

Copy link
Copy Markdown
Contributor

PR looks good. There's a problem with CircleCI here. Not sure why it isn't running. Because of that I can't merge either. Do you wanna try renaming the commit message? Perhaps that'll trigger CI. Use the PR title.

@remicalixte

Copy link
Copy Markdown
Author

Maybe CircleCI isn't triggered because I made the PR from a fork. I can't create a branch on the main repo

@gbbr

gbbr commented Oct 10, 2018 via email

Copy link
Copy Markdown
Contributor

@palazzem
palazzem force-pushed the remicalixte/fix-journald branch from 98bd8a0 to 9e1f171 Compare October 11, 2018 12:17
@palazzem

Copy link
Copy Markdown

@gbbr CircleCI is fine now. Let's wait for the other PR to be merged before pushing this in our 6.6.0. I want 1-1 feature parity with the core Agent.

@remicalixte
remicalixte force-pushed the remicalixte/fix-journald branch 2 times, most recently from 2b6d7ae to 5543cf0 Compare October 11, 2018 17:45
@remicalixte remicalixte changed the title flags: add flag to allow ignoring SIGPIPE signals ignore SIGPIPE signals Oct 11, 2018
@remicalixte

Copy link
Copy Markdown
Author

I think the fail in circleci is unrelated to the latest changes

@remicalixte

remicalixte commented Oct 11, 2018

Copy link
Copy Markdown
Author

DataDog/datadog-agent#2416 is merged, we could merge this now

@gbbr gbbr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Slight nit about the comment. Will merge after.

Comment thread cmd/trace-agent/main.go Outdated
log.Infof("received signal %d (%v)", signo, signo)
onSignal()
return
// By default systemd redirects the stdout to journald. When journald is stopped or crashes we receive a SIGPIPE signal.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please move this comment inside the case statement and remove the other one (// do nothing)

@gbbr gbbr changed the title ignore SIGPIPE signals cmd/trace-agent: ignore SIGPIPE signal Oct 12, 2018
@gbbr

gbbr commented Oct 12, 2018

Copy link
Copy Markdown
Contributor

Please consider my comment in the other PR: DataDog/datadog-agent#2416 (comment)

@remicalixte
remicalixte force-pushed the remicalixte/fix-journald branch from caf0b0c to 807c44b Compare October 12, 2018 14:55
@gbbr
gbbr merged commit f9fab3b into DataDog:master Oct 12, 2018
@remicalixte
remicalixte deleted the remicalixte/fix-journald branch October 12, 2018 16:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants