-
Notifications
You must be signed in to change notification settings - Fork 296
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
Fix user_event test cases #2368
Fix user_event test cases #2368
Conversation
…'m not able to reproduce the failures locally. Not sure why they were failing before.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2368 +/- ##
==========================================
- Coverage 73.91% 67.97% -5.95%
==========================================
Files 267 361 +94
Lines 9615 13324 +3709
==========================================
+ Hits 7107 9057 +1950
- Misses 2508 4267 +1759 Flags with carried forward coverage won't be shown. Click here to find out more. |
…est: Requested SDK version: 9.0.100 global.json file: /home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json Installed SDKs: Install the [9.0.100] .NET SDK or update [/home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json] to match an installed SDK.
@@ -104,6 +104,8 @@ jobs: | |||
with: | |||
project-name: Component[OpenTelemetry.Exporter.Geneva] | |||
code-cov-name: Exporter.Geneva | |||
os-list: '[ "ubuntu-24.04" ]' # Note: This may be switched to latest once ubuntu-latest has a kernel version >= 6.8.0-1014-azure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xiang17 Just some context for you. From what I could tell, user_events were added to the kernel. But early versions seemed to have an issue where user space couldn't register the tracepoint as was intended. That was later fixed. So you need a kernel with user_events enabled AND the permissions fix for it all to work correctly.
How it should work is the reader process (think agent/collector) needs sudo/admin. But the writer process (app emitting telemetry) shouldn't need anything special.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context. I simply restored your code back which worked in your PR.
I removed your code because I read it wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, simply adding back didn't work. The failure symptom is that the SDK is missing even though a correct SDK (9.0.101) is installed during setup dotnet step and dotnet restore & build steps were working:
sudo -E dotnet test ./build/Projects/Component.proj --collect:"Code Coverage" --results-directory:TestResults --framework net9.0 --configuration Release --no-restore --no-build --logger:"console;verbosity=detailed" --filter "CategoryName=Geneva:user_events:metrics" -- RunConfiguration.DisableAppDomain=true && sudo chmod a+rw ./TestResults
shell: /usr/bin/bash -e {0}
env:
BUILD_COMPONENT: OpenTelemetry.Exporter.Geneva
DOTNET_ROOT: /usr/share/dotnet
The command could not be loaded, possibly because:
* You intended to execute a .NET application:
The application 'test' does not exist.
* You intended to execute a .NET SDK command:
A compatible .NET SDK was not found.
Requested SDK version: 9.0.100
global.json file: /home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json
Installed SDKs:
Install the [9.0.100] .NET SDK or update [/home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json] to match an installed SDK.
Learn about SDK resolution:
https://aka.ms/dotnet/sdk-not-found
8.0.110 [/usr/lib/dotnet/sdk]
Error: Process completed with exit code 145.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I even changed the global.json version to 9.0.101 in case the installed 9.0.101 does not satisfy the global.json
's request for 9.0.100, however it still couldn't find the SDK:
sudo -E dotnet test ./build/Projects/Component.proj --collect:"Code Coverage" --results-directory:TestResults --framework net9.0 --configuration Release --no-restore --no-build --logger:"console;verbosity=detailed" --filter "CategoryName=Geneva:user_events:metrics" -- RunConfiguration.DisableAppDomain=true && sudo chmod a+rw ./TestResults
shell: /usr/bin/bash -e {0}
env:
BUILD_COMPONENT: OpenTelemetry.Exporter.Geneva
DOTNET_ROOT: /usr/share/dotnet
The command could not be loaded, possibly because:
* You intended to execute a .NET application:
The application 'test' does not exist.
* You intended to execute a .NET SDK command:
A compatible .NET SDK was not found.
Requested SDK version: 9.0.101
global.json file: /home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json
Installed SDKs:
8.0.110 [/usr/lib/dotnet/sdk]
Install the [9.0.101] .NET SDK or update [/home/runner/work/opentelemetry-dotnet-contrib/opentelemetry-dotnet-contrib/global.json] to match an installed SDK.
Learn about SDK resolution:
https://aka.ms/dotnet/sdk-not-found
Error: Process completed with exit code 145.
…est by removing sudo
Thanks! I did some tests and found that I poked the environment variables and tried manually adding All the
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Fixes #2326.
Added back tests which were failing on Linux after bumping .NET SDK to 9.0.100. I'm not able to reproduce the failures locally. Not sure why they were failing before.
Fixes #
Design discussion issue #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes