Clarify values for code.function.name and code.namespace
#1677
Comments
|
Wouldn't it be better/simpler to just have one attribute
Maybe we should name such attribute Related comment: #1624 (comment) |
|
Having a "do it all" attribute might make sense, but I think that we need to first gather what would work best for each platform as separate fields, then we might merge them if that's a better approach. Here there might not be a single "best solution" as the values are platform/language dependent so choosing any is always a compromise. |
|
For Java, what I think are the expected values for those attributes is the following as captured with this gist using the reflection API. When using the reflection API, the values for Within bytebuddy advices, we can also get those through So in the case of Java, I think that having separate attributes is probably the best option. |
|
For Python we are sending the following attributes in logs: Values are taken from python logging.LogRecord and in practice they would look like: |
|
@open-telemetry/dotnet-approvers could you help us out and post a common example(s) of what would be most common/expected to capture in your language for |
|
For PHP, we extensive use these for auto-instrumentation of a function or method call. For methods, From one of our tests, I don't see any issue with us merging these into a single field. |
|
Erlang/Elixir would be fully qualified module name for namespace and function/arity for function name. Elixir example:
Erlang
|
|
For Node.js/JavaScript: tl;drThere isn't any current normative usage in current OTel JS, so this is just my opinion. :) Examples: or perhaps that more details(Sorry this got long.) Current state: There is only one instrumentation in opentelemetry-js-contrib.git that is using If OTel instrumentation were to collect code location information, I think it would be from an In general in JavaScript, So, theoretically the OTel JS SDK could install a custom ... something like this modified "go-boom.js"// CallSite API from v8: https://v8.dev/docs/stack-trace-api#customizing-stack-traces
const orig = Error.prepareStackTrace ?? (() => {});
Error.prepareStackTrace = function (err, stack) {
const callsite = stack[0];
console.log('--');
console.log('code.file.path:', callsite.getFileName());
console.log('code.function.name:', callsite.getFunctionName());
console.log('code.line.number:', callsite.getLineNumber());
console.log('code.column.number:', callsite.getColumnNumber());
console.log('--');
return orig(err, stack);
}
function foo() {
throw new Error('boom');
}
foo();the result of which would be: This example uses the older CommonJS module system. With the newer ES Modules system that So, in general, the typical
|
|
Go: I it worth mentioning that the Go operates on fully qualified names (e.g. // splitFuncName splits package path-qualified function name into
// function name and package full name (namespace). E.g. it splits
// "github.com/my/repo/pkg.foo" into
// "foo" and "github.com/my/repo/pkg".
func splitFuncName(f string) (funcName, pkgName string) {
i := strings.LastIndexByte(f, '.')
if i < 0 {
return "", ""
}
return f[i+1:], f[:i]
} |
I would like to add that for PHP there might be few cases: For a function defined in an anonymous namespace: For a function defined in a named namespace: In this case, code.namespace will always be missing. In my opinion, this reveals an inconsistency in the interpretation of what constitutes a "namespace" versus a "function". This is due to the way PHP stores function names - introducing a split would create additional overhead. For a class method defined in an anonymous namespace: For a class method defined in a named namespace: |
|
@intuibase do we have an idea of the overhead that splitting would cause here ? For Go the conclusion was that it is negligible, but not zero. The original intent here is to favor consistency, but if the overhead becomes too important then we could keep some per-platform inconsistencies and document them. |
|
@SylvainJuge It all depends on the application, but in the real world, the overhead should be imperceptible. |
|
Rust: Using this playground as an example https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3196b0d9d24d31c3dafa15cb6769e9f1 The backtrace looks something like this: So the value should map like this: The context is that crates have a name which is included. Reference: https://doc.rust-lang.org/reference/names/namespaces.html |
|
interestingly, the profiling signal appears to be modeling "namespace + function" as a single field: there could be benefit in alignment it may help to also consider if there's alignment to be had with (future) open-telemetry/opentelemetry-specification#2839 |
|
@trask thanks for bringing this, having alignment could be worth switching to a single string field for the function name and deprecate In open-telemetry/opentelemetry-specification#2839 the issue is about the definition of a structured cross-language stacktrace format, and even if the discussion is about the much larger challenge of "string stacktraces" vs "structured stacktraces" we can learn a few things from it, in particular complexity of defining cross-language structure to describe code. For the profiling signal, do we have an available specification of the content of those attributes ? For example in Java it could also include the method arguments, so even if there is a single field in the profiling protobuf it would likely have to be split in for example |
@open-telemetry/profiling-approvers do you have any spec or examples for what is put into the |
We don't have a fixed format, so in practice it's whatever the unwinder/symbolizer for a specific runtime can produce. We don't have a separate notion for a namespace, instead it's included in the function name. Some examples (as you'll see, fidelity varies with Hotspot being the most comprehensive as it also includes argument/return types): Kernel frames
Go frames
Hotspot frames
Ruby frames
Python frames
|
|
Thanks @christos68k, this pushes a bit more on having a single attribute for the function name. For now, this single "function name" field is in the profiling data model (defined in protobuf), and the value really depends on the implementation of the unwinder/symbolizer, which brings at least two ideas/questions: First, aligning perfectly on the values in profiling and semconv for every platform/language might be desirable, but hard to achieve. In particular if we take the constraints and implementation details of the unwinder/symbolizer having to normalize and/or split fields might be tricky. However, doing some post-processing where this profiling data is produced seems possible. Then, the profiling data model expressed in the protobuf is mostly a "wire protocol" in the sense that it reflects how the data is being captured and stored efficiently, and as such it would be hard to add extra constraints from the semantic conventions on how things should be structured. What I think we need to provide is the ability to correlate the values in "profiling protobuf" and "code.* semantic conventions", even if that means 1 field in profiling would be split in multiple ones in semconv. If we take the example of the JVM, we can see that the profiling function name contains also the arguments and return value types, and currently we don't have any semconv attributes to store them (as a complete method signature or as individual attributes), so the best we could do for now is a partial match between semconv attributes and profiling data. |
I never studied compilers/interpreters but maybe there is some better name like |
This is part of #1599 where we aim to make
code.*attributes as release-candidate.Some of the
code.*attributes are being renamed with #1624, but this issue is about the values of those attributes and having a clear definition for them, in particular:code.function.name(previouslycode.function).code.namespaceIn the discussion of #1624 we found that not having explicit per-language examples leaves interpretation open (here and here), and is likely to cause ambiguity and potential inconsistencies, also there might be per-language constraints or overhead to provide those values.
Those attributes are currently in
experimental, and we aim to promote them torelease candidatewith #1599, as per this comment we consider that they are not used enough to justify a migration plan to prevent unexpected breaking changes (for example withOTEL_SEMCONV_STABILITY_OPT_INenvironment variable).Here the goal would be to define for each language:
code.function.namecode.namespaceFor now, we aim to maximize consistency, however if the overhead to provide those values (for example due to extra allocation or string splitting overhead), it might be possible to provide "slightly inconsistent values" to avoid those.
Checklist
The text was updated successfully, but these errors were encountered: