Skip to content

[Profiler] Add new profile tags#3093

Merged
chrisnas merged 13 commits into
masterfrom
chrisnas/add_dotnet_version_as_tag
Aug 23, 2022
Merged

[Profiler] Add new profile tags#3093
chrisnas merged 13 commits into
masterfrom
chrisnas/add_dotnet_version_as_tag

Conversation

@chrisnas

@chrisnas chrisnas commented Aug 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary of changes

When uploading a .pprof , add

  • .NET version as "runtime_version" tag (framework-4.8 or core-6.0 for example)
  • OS as "runtime_os" tag (linux or windows)
  • list of enabled profilers tag (walltime_cpu_exceptions)

Reason for change

Figure out which version of .NET is running profiled applications on which OS

Implementation details

Use ICorProfilerInfo::GetRuntimeInformation() to get major/minor versions with Framework or Core

Test coverage

Tags generation

Other details

@chrisnas
chrisnas requested a review from a team as a code owner August 17, 2022 11:55
@github-actions github-actions Bot added the area:profiler Issues related to the continous-profiler label Aug 17, 2022
@chrisnas chrisnas changed the title [Profiler] Add dotnet version as profile tag [Profiler] Add new profile tags Aug 17, 2022
gleocadie
gleocadie reviewed Aug 17, 2022
View reviewed changes
Comment thread profiler/src/ProfilerEngine/Datadog.Profiler.Native/LibddprofExporter.cpp Outdated
gleocadie
gleocadie approved these changes Aug 18, 2022
View reviewed changes

@gleocadie gleocadie left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

one comment otherwise LGTM

Comment thread profiler/src/ProfilerEngine/Datadog.Profiler.Native/LibddprofExporter.cpp Outdated
andrewlock
andrewlock reviewed Aug 23, 2022
View reviewed changes

@andrewlock andrewlock left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should wait for a manual run of the profiler integration tests before merging this - I've just triggered one

Are these tags standardised for the profiler, as they differ significantly from the values used in the tracer.

For the tracer, for example, we send the following headers:

Datadog-Meta-Lang-Interpreter: .NET Framework/.NET Core/.NET
Datadog-Meta-Lang-Version: 4.8.1/2.1/6.0

(we don't track os on spans directly AFAICT).

CI-visibility tags a lot more with span-level tags, e.g:
runtime.name: .NET Framework
runtime.version: 4.8.1
os.platform: Windows
os.version:

If these aren't standardised for profiling, maybe we should look at not introducing a whole new set of descriptions?

@chrisnas

chrisnas commented Aug 23, 2022

Copy link
Copy Markdown
Contributor Author

You should wait for a manual run of the profiler integration tests before merging this - I've just triggered one

Thanks!
I'm struggling with the CI since last week...

Are these tags standardised for the profiler, as they differ significantly from the values used in the tracer.

For the tracer, for example, we send the following headers:

Datadog-Meta-Lang-Interpreter: .NET Framework/.NET Core/.NET Datadog-Meta-Lang-Version: 4.8.1/2.1/6.0

(we don't track os on spans directly AFAICT).

CI-visibility tags a lot more with span-level tags, e.g: runtime.name: .NET Framework runtime.version: 4.8.1 os.platform: Windows os.version:

If these aren't standardised for profiling, maybe we should look at not introducing a whole new set of descriptions?

They are defined among profiler runtimes already.
I don't know why the tracer tags were not used :^(

@chrisnas
chrisnas merged commit 41bbf3f into master Aug 23, 2022
@chrisnas
chrisnas deleted the chrisnas/add_dotnet_version_as_tag branch August 23, 2022 14:05
@github-actions github-actions Bot added this to the vNext milestone Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewlock andrewlock andrewlock left review comments
@gleocadie gleocadie gleocadie approved these changes

Assignees

No one assigned

Labels

area:profiler Issues related to the continous-profiler

Projects

None yet

Milestone

2.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@chrisnas @gleocadie @andrewlock