-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[receiver/hostmetrics] Cheaper parent PID and number of threads retrieval on Windows #38589
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
Merged
dmitryax
merged 14 commits into
open-telemetry:main
from
pjanotti:win-hostmetricsreceiver-cheaper-get-parent-pid
Mar 21, 2025
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b00fe53
Prototyping cheaper parent PID retrieval on Windows
pjanotti ad957d5
Lint
pjanotti febe799
Polishing per initial feedback
pjanotti a4cd0bc
Merge branch 'main' into win-hostmetricsreceiver-cheaper-get-parent-pid
pjanotti d4afa91
changelog
pjanotti 6cf9009
Change the featuregate
pjanotti 89700f4
Clean initialization of wrappedProcessHandle
pjanotti 5e37d3a
Merge branch 'main' into win-hostmetricsreceiver-cheaper-get-parent-pid
pjanotti cb86156
Fix formatting
pjanotti 842dee2
Merge branch 'main' into win-hostmetricsreceiver-cheaper-get-parent-pid
pjanotti 9ad27b6
Merge branch 'main' into win-hostmetricsreceiver-cheaper-get-parent-pid
pjanotti 4ae28a7
Fix casing of feature flag
pjanotti 24577fd
Merge branch 'main' into win-hostmetricsreceiver-cheaper-get-parent-pid
pjanotti d43aa79
Remove the extra MuteProcessAllErrors added for easy benchmarking
pjanotti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
29 changes: 29 additions & 0 deletions
29
.chloggen/hostmetricsreceiver-process-scraper-windows-optimization.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: 'enhancement' | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) | ||
component: hostmetricsreceiver | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: > | ||
Reduced the cost of retrieving number of threads and parent process ID on Windows. | ||
Disable the featuregate `hostmetrics.process.onWindowsUseNewGetProcesses` to fallback to the previous implementation. | ||
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
issues: [32947, 38589] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: | ||
|
||
# If your change doesn't affect end users or the exported elements of any package, | ||
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
# Optional: The change log or logs in which this entry should be included. | ||
# e.g. '[user]' or '[user, api]' | ||
# Include 'user' if the change is relevant to end users. | ||
# Include 'api' if there is a change to a library API. | ||
# Default: '[user]' | ||
change_logs: [user] |
27 changes: 27 additions & 0 deletions
27
receiver/hostmetricsreceiver/internal/scraper/processscraper/get_process_handles_others.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !windows | ||
|
||
package processscraper // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/processscraper" | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/shirou/gopsutil/v4/process" | ||
) | ||
|
||
func getGopsutilProcessHandles(ctx context.Context) (processHandles, error) { | ||
processes, err := process.ProcessesWithContext(ctx) | ||
if err != nil { | ||
return nil, err | ||
} | ||
wrapped := make([]wrappedProcessHandle, len(processes)) | ||
for i, p := range processes { | ||
wrapped[i] = wrappedProcessHandle{ | ||
Process: p, | ||
} | ||
} | ||
|
||
return &gopsProcessHandles{handles: wrapped}, nil | ||
} |
84 changes: 84 additions & 0 deletions
84
receiver/hostmetricsreceiver/internal/scraper/processscraper/get_process_handles_windows.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build windows | ||
|
||
package processscraper // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver/internal/scraper/processscraper" | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"unsafe" | ||
|
||
"github.com/shirou/gopsutil/v4/process" | ||
"go.opentelemetry.io/collector/featuregate" | ||
"golang.org/x/sys/windows" | ||
) | ||
|
||
var useNewGetProcessHandles = featuregate.GlobalRegistry().MustRegister( | ||
"hostmetrics.process.onWindowsUseNewGetProcesses", | ||
featuregate.StageBeta, | ||
featuregate.WithRegisterDescription("If disabled, the scraper will use the legacy implementation to retrieve process handles."), | ||
) | ||
|
||
func getGopsutilProcessHandles(ctx context.Context) (processHandles, error) { | ||
if !useNewGetProcessHandles.IsEnabled() { | ||
return getGopsutilProcessHandlesLegacy(ctx) | ||
} | ||
|
||
snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, 0) | ||
if err != nil { | ||
return nil, fmt.Errorf("could not create snapshot: %w", err) | ||
} | ||
defer func() { | ||
_ = windows.CloseHandle(snap) | ||
}() | ||
|
||
var pe32 windows.ProcessEntry32 | ||
pe32.Size = uint32(unsafe.Sizeof(pe32)) | ||
if err = windows.Process32First(snap, &pe32); err != nil { | ||
return nil, fmt.Errorf("could not get first process: %w", err) | ||
} | ||
|
||
wrappedProcesses := make([]wrappedProcessHandle, 0, 64) | ||
for { | ||
select { | ||
case <-ctx.Done(): | ||
return nil, ctx.Err() | ||
default: | ||
// Ignoring any errors here to keep same behavior as the legacy implementation | ||
// based on the `process.ProcessesWithContext` from the `gopsutil` package. | ||
p, _ := process.NewProcess(int32(pe32.ProcessID)) | ||
if p != nil { | ||
wrappedProcess := wrappedProcessHandle{ | ||
Process: p, | ||
parentPid: int32(pe32.ParentProcessID), | ||
initialNumThreads: int32(pe32.Threads), | ||
flags: flagParentPidSet | flagUseInitialNumThreadsOnce, | ||
} | ||
wrappedProcesses = append(wrappedProcesses, wrappedProcess) | ||
} | ||
} | ||
|
||
if err = windows.Process32Next(snap, &pe32); err != nil { | ||
break | ||
} | ||
} | ||
|
||
return &gopsProcessHandles{handles: wrappedProcesses}, nil | ||
} | ||
|
||
func getGopsutilProcessHandlesLegacy(ctx context.Context) (processHandles, error) { | ||
processes, err := process.ProcessesWithContext(ctx) | ||
if err != nil { | ||
return nil, err | ||
} | ||
wrapped := make([]wrappedProcessHandle, len(processes)) | ||
for i, p := range processes { | ||
wrapped[i] = wrappedProcessHandle{ | ||
Process: p, | ||
} | ||
} | ||
|
||
return &gopsProcessHandles{handles: wrapped}, nil | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 65 additions & 0 deletions
65
...er/hostmetricsreceiver/internal/scraper/processscraper/process_metadata_benchmark_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build windows | ||
|
||
package processscraper | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file could be removed when we remove the featureflag to fallback to the old behavior. |
||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
"go.opentelemetry.io/collector/featuregate" | ||
"go.opentelemetry.io/collector/scraper" | ||
) | ||
|
||
func BenchmarkGetProcessMetadata(b *testing.B) { | ||
ctx := context.Background() | ||
config := &Config{} | ||
|
||
scraper, err := newProcessScraper(scraper.Settings{}, config) | ||
if err != nil { | ||
b.Fatalf("Failed to create process scraper: %v", err) | ||
} | ||
|
||
benchmarks := []struct { | ||
name string | ||
useLegacy bool | ||
parentPidEnabled bool | ||
}{ | ||
{ | ||
name: "New-ExcludeParentPid", | ||
}, | ||
{ | ||
name: "Old-ExcludeParentPid", | ||
useLegacy: true, | ||
}, | ||
{ | ||
name: "New-IncludeParentPid", | ||
parentPidEnabled: true, | ||
}, | ||
{ | ||
name: "Old-IncludeParentPid", | ||
parentPidEnabled: true, | ||
useLegacy: true, | ||
}, | ||
} | ||
|
||
for _, bm := range benchmarks { | ||
b.Run(bm.name, func(b *testing.B) { | ||
// Set feature gate value | ||
previousValue := useNewGetProcessHandles.IsEnabled() | ||
require.NoError(b, featuregate.GlobalRegistry().Set(useNewGetProcessHandles.ID(), !bm.useLegacy)) | ||
defer func() { | ||
require.NoError(b, featuregate.GlobalRegistry().Set(useNewGetProcessHandles.ID(), previousValue)) | ||
}() | ||
scraper.config.MetricsBuilderConfig.ResourceAttributes.ProcessParentPid.Enabled = bm.parentPidEnabled | ||
|
||
for i := 0; i < b.N; i++ { | ||
// Typically there are errors, but we are not interested in them for this benchmark | ||
_, _ = scraper.getProcessMetadata(ctx) | ||
} | ||
}) | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.