Support span attached event concept#9916
Conversation
mrproliu
added
protocol
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| private org.apache.skywalking.oap.server.core.query.type.SpanAttachedEvent parseEvent(SpanAttachedEvent event) { | ||
| final org.apache.skywalking.oap.server.core.query.type.SpanAttachedEvent result = | ||
| new org.apache.skywalking.oap.server.core.query.type.SpanAttachedEvent(); | ||
| result.getStartTime().setSeconds(event.getStartTime().getSeconds()); |
There was a problem hiding this comment.
NULL_DEREFERENCE: object returned by getStartTime(result) could be null and is dereferenced at line 315.
ℹ️ Learn about @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
| @Getter | ||
| @ScopeDeclaration(id = SPAN_ATTACHED_EVENT, name = "SpanAttachedEvent") | ||
| @Stream(name = SpanAttachedEventRecord.INDEX_NAME, scopeId = SPAN_ATTACHED_EVENT, builder = SpanAttachedEventRecord.Builder.class, processor = RecordStreamProcessor.class) | ||
| public class SpanAttachedEventRecord extends Record { |
There was a problem hiding this comment.
Are you going to use 100% sampling? This means sampling all events matching the trace header. If so, consider @SuperDataset.
There was a problem hiding this comment.
Updated it as super data set.
| log.debug("receive span attached event is streaming"); | ||
| } | ||
|
|
||
| final SpanAttachedEventRecord record = new SpanAttachedEventRecord(); |
There was a problem hiding this comment.
Should we consider HTTP endpoint?
There was a problem hiding this comment.
Because there is no need now, so ignore it.
| @Column(columnName = END_TIME_NANOS) | ||
| private int endTimeNanos; | ||
| @Column(columnName = TRACE_REF_TYPE) | ||
| private int traceRefType; |
There was a problem hiding this comment.
Should we add a SpanReferenceType type field(not a column` in the Record?
There was a problem hiding this comment.
Updated it as an enum.
...apache/skywalking/oap/server/core/analysis/manual/spanattach/SpanAttachedEventTraceType.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // find matches span | ||
| final int eventSpanId = Integer.parseInt(record.getTraceSpanId()); |
There was a problem hiding this comment.
I think we should check SpanReferenceType first, then do casting. Otherwise, we may face NPE or cast error, if trace ID actually exists on both sides. Such as Lua agent uses UUID as well.
There was a problem hiding this comment.
According to this, apache/skywalking-query-protocol#101, we should update the Zipkin query as well. And verify whether we could put this in tags or some places for lens UI?
There was a problem hiding this comment.
Updated. I have filtered the skywalking events in the query and checked the parent span ID is numeric or not.
docs/en/changes/changes.md
Outdated
| * Optimize MQ Topology analysis. Use entry span's peer from the consumer side as source service when no producer instrumentation(no cross-process reference). | ||
| * Refactor JDBC storage implementations to reuse logics. | ||
| * Fix `ClassCastException` in `LoggingConfigWatcher`. | ||
| * Support span attached event concept. |
There was a problem hiding this comment.
| * Support span attached event concept. | |
| * Support span attached event concept in Zipkin and SkyWalking trace query. | |
| * Support span attached events on Zipkin lens UI. |
This doc should be updated as well, https://skywalking.apache.org/docs/main/next/en/setup/backend/zipkin-trace/
Also, we should consider adding a menu here to introduce ebpf besides tracing/logging/metrics.
There was a problem hiding this comment.
Let's update the docs for profiling in the next PR.
2 participants
Support receiving, storing, and querying the span attached event in tracing.
CHANGESlog.