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

reporter: use semantic convention to report frame type #167

Merged
merged 7 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ devfiler spins up a local server that listens on `0.0.0.0:11000`.

To run it, simply download and unpack the archive from the following URL:

https://upload.elastic.co/d/a3033fd2b515144b1c566961495302dac4da3223e59832f7755e18ac7fd94a19
https://upload.elastic.co/d/87e7697991940ec37f0c6e39ac38d213f65e8dc1ef9dbedff6aab9cba0adfaba

Authentication token: `c74dfc4db2212015`

Authentication token: `786a077d31b02bda`

The archive contains a build for each of the following platforms:

Expand Down
5 changes: 0 additions & 5 deletions libpf/frametype.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package libpf // import "go.opentelemetry.io/ebpf-profiler/libpf"

import (
"fmt"

"go.opentelemetry.io/ebpf-profiler/support"
)

Expand Down Expand Up @@ -98,9 +96,6 @@ func (ty FrameType) String() string {
return abortFrameName
default:
interp := ty.Interpreter()
if ty.IsError() {
return fmt.Sprintf("%s-error", interp)
}
return interp.String()
}
}
8 changes: 5 additions & 3 deletions libpf/frametype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ func TestFrameTypeFromString(t *testing.T) {
for _, ft := range []FrameType{
unknownFrame, PHPFrame, PythonFrame, NativeFrame, KernelFrame, HotSpotFrame, RubyFrame,
PerlFrame, V8Frame, DotnetFrame, AbortFrame} {
name := ft.String()
result := FrameTypeFromString(name)
require.Equal(t, ft, result)
t.Run(ft.String(), func(t *testing.T) {
name := ft.String()
result := FrameTypeFromString(name)
require.Equal(t, ft, result)
})
}
}
24 changes: 14 additions & 10 deletions libpf/interpretertype.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,26 @@ func (i InterpreterType) Frame() FrameType {
var interpreterTypeToString = map[InterpreterType]string{
UnknownInterp: "unknown",
PHP: "php",
PHPJIT: "phpjit",
Python: "python",
Native: "native",
Kernel: "kernel",
HotSpot: "jvm",
Ruby: "ruby",
Perl: "perl",
V8: "v8",
Dotnet: "dotnet",
APMInt: "apm-integration",
// OTel SemConv does not differentiate between jitted code and interpreted code.
PHPJIT: "php",
Python: "cpython",
Native: "native",
Kernel: "kernel",
HotSpot: "jvm",
Ruby: "ruby",
Perl: "perl",
V8: "v8js",
Dotnet: "dotnet",
APMInt: "apm-integration",
}

var stringToInterpreterType = make(map[string]InterpreterType, len(interpreterTypeToString))

func init() {
for k, v := range interpreterTypeToString {
if k == PHPJIT {
continue
}
stringToInterpreterType[v] = k
}
}
Expand Down
4 changes: 2 additions & 2 deletions libpf/libpf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ func TestTraceType(t *testing.T) {
ty: PythonFrame,
isErr: false,
interp: Python,
str: "python",
str: "cpython",
},
{
ty: NativeFrame.Error(),
isErr: true,
interp: Native,
str: "native-error",
str: "native",
},
}

Expand Down
8 changes: 5 additions & 3 deletions reporter/otlp_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,13 +564,15 @@ func (r *OTLPReporter) getProfile() (profile *profiles.Profile, startTS, endTS u

// Walk every frame of the trace.
for i := range traceInfo.frameTypes {
frameAttributes := addProfileAttributes(profile, []attrKeyValue{
Copy link
Member

@christos68k christos68k Oct 3, 2024

Choose a reason for hiding this comment

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

This allows "abort-marker" (abortFrameName) to be a value that the agent sends with profile.frame.type, but there's no semantic convention for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@christos68k Does it make sense to create a separate issue for this, so we can merge this PR. Without it, the devfiler won't work with the latest agent.

{key: "profile.frame.type", value: traceInfo.frameTypes[i].String()},
}, attributeMap)

loc := &profiles.Location{
// Id - Optional element we do not use.
TypeIndex: getStringMapIndex(stringMap,
traceInfo.frameTypes[i].String()),
Address: uint64(traceInfo.linenos[i]),
// IsFolded - Optional element we do not use.
// Attributes - Optional element we do not use.
Attributes: frameAttributes,
}

switch frameKind := traceInfo.frameTypes[i]; frameKind {
Expand Down