FrankinRUC commented on a change in pull request #217:
URL: 
https://github.com/apache/incubator-yunikorn-core/pull/217#discussion_r514956324



##########
File path: pkg/trace/scheduler_trace_context.go
##########
@@ -0,0 +1,186 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package trace
+
+import (
+       "fmt"
+       "time"
+
+       "github.com/opentracing/opentracing-go"
+       "github.com/opentracing/opentracing-go/ext"
+       "github.com/uber/jaeger-client-go"
+)
+
+// SchedulerTraceContext manages spans for one trace.
+// it only designs for the scheduling process so we keeps the interface simple
+type SchedulerTraceContext interface {
+       // ActiveSpan returns current active (latest unfinished) span in this 
context object.
+       // Error returns if there doesn't exist an unfinished span.
+       ActiveSpan() (opentracing.Span, error)
+
+       // StartSpan creates and starts a new span based on the context state 
with the operationName parameter.
+       // The new span is the child of current active span if it exists.
+       // Or the new span will become the root span of this trace.
+       StartSpan(operationName string) (opentracing.Span, error)
+
+       // FinishActiveSpan finishes current active span and set its parent as 
active if exists.
+       // Error returns if there doesn't exist an unfinished span.
+       FinishActiveSpan() error
+}
+
+var _ SchedulerTraceContext = &SchedulerTraceContextImpl{}
+
+// SchedulerTraceContextImpl reports the spans to tracer once they are 
finished.
+// Root span's "sampling.priority" tag will be set to 1 to force reporting all 
spans if OnDemandFlag is true.
+type SchedulerTraceContextImpl struct {
+       Tracer       opentracing.Tracer
+       SpanStack    []opentracing.Span
+       OnDemandFlag bool
+}
+
+func (s *SchedulerTraceContextImpl) ActiveSpan() (opentracing.Span, error) {
+       if len(s.SpanStack) == 0 {
+               return nil, fmt.Errorf("active span is not found")
+       }
+       return s.SpanStack[len(s.SpanStack)-1], nil
+}
+
+func (s *SchedulerTraceContextImpl) StartSpan(operationName string) 
(opentracing.Span, error) {
+       var newSpan opentracing.Span
+       if span, err := s.ActiveSpan(); err != nil {
+               newSpan = s.Tracer.StartSpan(operationName)
+               if s.OnDemandFlag {
+                       ext.SamplingPriority.Set(newSpan, 1)
+               }
+       } else {
+               newSpan = s.Tracer.StartSpan(operationName, 
opentracing.ChildOf(span.Context()))
+       }
+       s.SpanStack = append(s.SpanStack, newSpan)
+       return newSpan, nil
+}
+
+func (s *SchedulerTraceContextImpl) FinishActiveSpan() error {
+       span, err := s.ActiveSpan()
+       if err != nil {
+               return err
+       }
+       span.Finish()
+       s.SpanStack = s.SpanStack[:len(s.SpanStack)-1]
+       return nil
+}
+
+var _ opentracing.Span = &DelaySpan{}
+
+// DelaySpan implements the opentracing.Span interface.
+// It will set the FinishTime field and delay reporting when finished.

Review comment:
       Do you mean that we should keep a index pointing to the last unfinished 
(active) span and move it back and forth to find the new  active span when we 
start or finish spans? The stask is a space-time tradeoff. It records all the 
unfinished spans to save the time for finding the active span. We know that it 
will be more space comsuming than regular tracing method, so we only use this 
implementation if we need the scheduling result tags to find out whether we 
should collect this trace.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to