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.

Sign up for GitHub

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

use LinkedTo rather than ChildOf for PublicEndpoint #272

Merged
merged 3 commits into from
Nov 4, 2019
Merged

use LinkedTo rather than ChildOf for PublicEndpoint #272

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ac579f
use LinkedTo rather than ChildOf for PublicEndpoint
lizthegrey Nov 1, 2019
e15b34a
Merge branch 'master' into lizf.LinkTo
lizthegrey Nov 1, 2019
b91c3d0
Merge branch 'master' into lizf.LinkTo
bogdandrutu Nov 4, 2019
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
8 changes: 8 additions & 0 deletions api/trace/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ type SpanOption func(*SpanOptions)
type SpanOptions struct {
Attributes []core.KeyValue
StartTime time.Time
Links []Link
Relation Relation
Record bool
SpanKind SpanKind
Expand Down Expand Up @@ -197,6 +198,13 @@ func FollowsFrom(sc core.SpanContext) SpanOption {
}
}

// LinkedTo allows instantiating a Span with initial Links.
func LinkedTo(sc core.SpanContext, attrs ...core.KeyValue) SpanOption {
return func(o *SpanOptions) {
o.Links = append(o.Links, Link{sc, attrs})
}
}

// WithSpanKind specifies the role a Span on a Trace.
func WithSpanKind(sk SpanKind) SpanOption {
return func(o *SpanOptions) {
Expand Down
8 changes: 3 additions & 5 deletions plugin/othttp/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
if sc.IsValid() { // not a valid span context, so no link / parent relationship to establish
var opt trace.SpanOption
if h.public {
// TODO: If the endpoint is a public endpoint, it should start a new trace
// and incoming remote sctx should be added as a link
// (WithLinks(links...), this option doesn't exist yet). Replace ChildOf
// below with something like: opt = trace.WithLinks(sc)
opt = trace.ChildOf(sc)
// If the endpoint is a public endpoint, it should start a new trace
// and incoming remote sctx should be added as a link.
opt = trace.LinkedTo(sc)
} else { // not a private endpoint, so assume child relationship
opt = trace.ChildOf(sc)
Copy link
Contributor

Choose a reason for hiding this comment

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

On this note, aren't we removing ChildOf? I'm on the fence about this, because we still haven't fully clarified SpanKind. open-telemetry/opentelemetry-specification#337

Copy link
Member Author

Choose a reason for hiding this comment

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

In either case we need a way to set the parent relationship, regardless of whether it's precisely specified according to SpanKind or more generic.

I am fine with punting this until we more fully remove ChildOf and just updating in place when we do so.

}
Expand Down
4 changes: 4 additions & 0 deletions sdk/trace/tracer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ func (tr *tracer) Start(ctx context.Context, name string, o ...apitrace.SpanOpti

spanName := tr.spanNameWithPrefix(name)
span := startSpanInternal(tr, spanName, parent, remoteParent, opts)
for _, l := range opts.Links {
span.AddLink(l)
}

span.tracer = tr

if span.IsRecording() {
Expand Down