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.

Sign up for GitHub

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

Jump to bottom

Debug #1803

Draft
wants to merge 5 commits into
base: release-3.19
Choose a base branch
from
Draft

Debug #1803

wants to merge 5 commits into from

Conversation

erthalion
Copy link
Contributor

@erthalion erthalion commented Aug 19, 2024 •
edited
Loading

Description

A dummy PR to create a debugging version of 3.19.2

@erthalion erthalion added the run-multiarch-builds Run steps for non-x86 archs. label Aug 19, 2024
@erthalion
Debug
481b4b4
@erthalion
Propagate config
c5dcf5b 
Pass CollectorConfig to the ProcessSignalHandler and
ProcessSignalFormatter. It's needed for the future work, involving
conditional logic in the process handler.
@erthalion
Allow to not collect process arguments
766154a
@erthalion
Add extended argument tests into the image
61089fe
Molter73
Molter73 reviewed Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

Looks good, if we were to merge it I would argue against store copies of the collector configuration in other objects, but for the purpose of troubleshooting it is good enough as is.

@erthalion
Copy link
Contributor Author

I would argue against store copies of the collector configuration in other objects

AFAICT it's passed by reference in this PR, why would there be any copying involved?

@Molter73
Copy link
Collaborator

I would argue against store copies of the collector configuration in other objects

AFAICT it's passed by reference in this PR, why would there be any copying involved?

It's passed by reference to the constructors, but it is stored as a concrete value in these 2 places:

collector/collector/lib/ProcessSignalFormatter.h

Line 57 in 61089fe

CollectorConfig config_;

collector/collector/lib/ProcessSignalHandler.h

Line 40 in 61089fe

CollectorConfig config_;

So you'll get copies in the end, if you delete the implicit copy constructor for CollectorConfig you'll see compilation fails on those two places and in CollectorService.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Molter73 Molter73 Molter73 left review comments

Assignees
No one assigned
Labels
do-not-merge/work-in-progress run-multiarch-builds Run steps for non-x86 archs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erthalion @Molter73