-
Notifications
You must be signed in to change notification settings - Fork 182
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
added mobile instrumentation to log-event semantic conventions #67
Conversation
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
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 think we've landed at a good first pass for this lifecycle event. Thanks for sticking it out and collaborating!
@LikeTheSalad looks like the PR needs a markdown lint run.
In your example, if you want to find out a crash happened after ON_PAUSE but before ON_STOP, it's probably better to log that as a breadcrumb trail of events on a Span that happens before a crash event. Plus you'll want the lifecycle owner name to do that kind of debugging, which isn't captured in the app state.
Just wanted to point out that this is essentially what we are currently doing in the Splunk android sdk -- we put Activity state change events as span events on the "active" span for each activity..
specification/logs/semantic_conventions/Instrumentation/mobile-events.md
Outdated
Show resolved
Hide resolved
So during the last Client SIG, folks were saying that the current direction is to model a session not using a container Span, but as an attribute set on the particular signal (implementation details to be decided). Is this something you're thinking of following for the Splunk/Android-Extension, @breedx-splk? |
Yes, I think android instrumentation will want to keep its telemetry looking like the spec has decided and what will hopefully one day soon match with web and ios. If the attribute gets decided, then we will go that route. In the splunk distro, we will probably need to maintain the use of zipkin spans in the short term until we can coordinate with our internal teams. |
@bryce-b Looks like a rebase is needed. Thx! |
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.
LGTM content wise.
A few things regarding the structure of that:
- Instead of writing the tables in the markdown manually let's define the attributes in a
yaml
file in themodel
directory. And then usemake table-generation
to generate the markdown tables in this file. - since this PR introduces a new semantic conventions area (i.e. directory under
docs
), we need aREADME.md
file in here (see other domain directories for comparison).
@open-telemetry/specs-semconv-approvers @open-telemetry/specs-semconv-maintainers @tigrannajaryan |
The PR contains a bunch of unrelated changes/commits. Please clean it up to make it reviewable. |
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
Co-authored-by: Alexander Wert <AlexanderWert@users.noreply.github.com>
@open-telemetry/specs-semconv-maintainers we have 4 approvals and the build is passing. Any reason not to merge? |
Adding this semantic convention for mobile events.