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

[Elastic Agent] The system process metricsets should not report as degraded when metrics are partially collected #40542

Closed
2 tasks done
cmacknz opened this issue Aug 15, 2024 · 7 comments · Fixed by #40565
Closed
2 tasks done
Assignees
Labels
Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Comments

@cmacknz
Copy link
Member

cmacknz commented Aug 15, 2024

In #40400 we added the ability for metricsets to report their status using the Elastic Agent control protocol to make errors more visible. One consequence of this has been surfacing previously hidden problems collecting information about PIDs.

It appears that by default we fail to get complete metrics for some PIDs on both Windows and Linux, resulting in the Elastic Agent being permanently unhealthy in the Fleet UI where no quick fix:

While this functionality is useful and has helped us find bugs, we do not want users to immediately experience unhealthy agents they can't obviously fix. As a stop gap while we fix the underlying problems, let's keep reporting the error message but have the system process metricsets report as healthy. This will put the errors messages in the UI without making the agent unhealthy, which most users treat as something to quickly fix.

For example we see something like the following in our leak detection tests regularly:

    - id: system/metrics-default
      state:
        message: 'Healthy: communicating with pid ''1556'''
        pid: 0
        state: 2
        units:
            input-system/metrics-default-system/metrics-system-5f5e65eb-2fd6-41e1-8c29-f24d57e66509:
                state: DEGRADED
                message: |-
                    Error fetching data for metricset system.process_summary: Not enough privileges to fetch information: Not enough privileges to fetch information: GetInfoForPid: could not get all information for PID 0: error fetching name: OpenProcess failed for pid=0: The parameter is incorrect.
                    error fetching status: OpenProcess failed for pid=0: The parameter is incorrect.
                    GetInfoForPid: could not get all information for PID 4: error fetching name: GetProcessImageFileName failed for pid=4: GetProcessImageFileName failed: invalid argument
                payload:
                    streams:
                        system/metrics-system.process.summary-5f5e65eb-2fd6-41e1-8c29-f24d57e66509:
                            error: |-
                                Error fetching data for metricset system.process_summary: Not enough privileges to fetch information: Not enough privileges to fetch information: GetInfoForPid: could not get all information for PID 0: error fetching name: OpenProcess failed for pid=0: The parameter is incorrect.
                                error fetching status: OpenProcess failed for pid=0: The parameter is incorrect.
                                GetInfoForPid: could not get all information for PID 4: error fetching name: GetProcessImageFileName failed for pid=4: GetProcessImageFileName failed: invalid argument
                            status: DEGRADED

The proposal is that we keep the error messages, but report the state as healthy.

    - id: system/metrics-default
      state:
        message: 'Healthy: communicating with pid ''1556'''
        pid: 0
        state: 2
        units:
            input-system/metrics-default-system/metrics-system-5f5e65eb-2fd6-41e1-8c29-f24d57e66509:
                state: HEALTHY
                message: |-
                    Error fetching data for metricset system.process_summary: Not enough privileges to fetch information: Not enough privileges to fetch information: GetInfoForPid: could not get all information for PID 0: error fetching name: OpenProcess failed for pid=0: The parameter is incorrect.
                    error fetching status: OpenProcess failed for pid=0: The parameter is incorrect.
                    GetInfoForPid: could not get all information for PID 4: error fetching name: GetProcessImageFileName failed for pid=4: GetProcessImageFileName failed: invalid argument
                payload:
                    streams:
                        system/metrics-system.process.summary-5f5e65eb-2fd6-41e1-8c29-f24d57e66509:
                            error: |-
                                Error fetching data for metricset system.process_summary: Not enough privileges to fetch information: Not enough privileges to fetch information: GetInfoForPid: could not get all information for PID 0: error fetching name: OpenProcess failed for pid=0: The parameter is incorrect.
                                error fetching status: OpenProcess failed for pid=0: The parameter is incorrect.
                                GetInfoForPid: could not get all information for PID 4: error fetching name: GetProcessImageFileName failed for pid=4: GetProcessImageFileName failed: invalid argument
                            status: HEALTHY

This will keep the information accessible for bug reports but should hopefully reduce the perceived urgency of the problem and the level of support cases. The control protocol always allows both a state and a message regardless of what the state is, see here.

There will be a follow up issue to make switching between these two.

@cmacknz cmacknz added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Aug 15, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@cmacknz
Copy link
Member Author

cmacknz commented Aug 15, 2024

I also think we should put this choice behind a module or metricset level configuration flag. I created #40543 to track implementing that.

@leehinman
Copy link
Contributor

Long term could we look at a way for users to configure that certain error conditions are acceptable or don't contribute to a DEGRADED state?

I'm thinking as a user I might want to say "Yes, I know we will never be able to collect process stats from process X, but any other process I want to know if there are errors"

@cmacknz
Copy link
Member Author

cmacknz commented Aug 15, 2024

Yes I think a granular filter would make sense, either as PIDs that contribute to errors or just an exclusion or inclusion list of process names/paths/pids to not even both collecting metrics from.

In the case of not having enough privileges, we would probably want to come up with well known error codes users can filter on (I'd probably just use the OS error codes e.g. EACCESS) rather that making our errors messages the API for this.

@ycombinator
Copy link
Contributor

Please remember to revert elastic/elastic-agent#5301 once this issue here is resolved.

@ycombinator
Copy link
Contributor

@VihasMakwana Now that you've resolved this issue via #40565, please remember to revert elastic/elastic-agent#5301, probably on Monday once the changes from #40565 have made it into a Beats snapshot release.

@VihasMakwana
Copy link
Contributor

VihasMakwana commented Aug 23, 2024

@ycombinator yes, that's on my list. I'll do that on Monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
5 participants