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

Turn off poison detection by default for AS/AT #484

Merged
merged 5 commits into from
Feb 3, 2022

Conversation

Lagovas
Copy link
Collaborator

@Lagovas Lagovas commented Feb 2, 2022

  • turned off poison record detection for server and translator
  • added integration tests for translator with poison record detection (were only for server)
  • added logging about turning on poison record detection at startup
  • fixed failure of prometheus tests due to port collision (works on CI because we re-define ports for acra-server)
  • changed a bit timeouts to use lower sleeps but same total timeout
  • removed tests for whole cell mode with poison records because this option ignored far ago
  • updated docs - Update default value for poison_detect_enable flag for acra-[server|translator] product-docs#230

Checklist

Copy link
Collaborator

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

👏

CHANGELOG_DEV.md Outdated Show resolved Hide resolved
CHANGELOG_DEV.md Outdated Show resolved Hide resolved
@@ -482,16 +482,19 @@ func realMain() error {

poisonCallbacks := poison.NewCallbackStorage()
if *detectPoisonRecords {
log.WithField(logging.FieldKeyEventCode, logging.EventCodePoisonRecordDetectionMessage).Infoln("Turned on poison record detection")
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 valuable logs, thanx!

os.Exit(1)
log.Errorln("executed code after os.Exit")
log.WithField(logging.FieldKeyEventCode, logging.EventCodePoisonRecordDetectionMessage).Errorln("executed code after os.Exit")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
log.WithField(logging.FieldKeyEventCode, logging.EventCodePoisonRecordDetectionMessage).Errorln("executed code after os.Exit")
log.WithField(logging.FieldKeyEventCode, logging.EventCodePoisonRecordDetectionMessage).Errorln("Executed code after os.Exit")

should it also start from the capital letter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, this code should not be executed because after os.Exit. It's just honeypot that we check in our integration tests that it's not executed and service stopped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i see

# how long wait forked process to respond
FORK_TIMEOUT = 2
# seconds for sleep call after failed polling forked process
FORK_FAIL_SLEEP = 0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Co-authored-by: vixentael <vixentael@users.noreply.github.com>
@Lagovas Lagovas merged commit d6908f5 into master Feb 3, 2022
@Lagovas Lagovas deleted the lagovas/T2439-turn-off-poison-detection-by-default branch February 3, 2022 12:18
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.

3 participants