Athens: Add tracing to functions with a context.Context

[tbpg]

See #174, which threads contexts throughout the project.

Starter list (there are more outside of storage):

• [x] storage.Exists
• [x] storage.Delete
• [x] storage.Get
• [x] storage.List
• [x] storage.Save

From @arschles on #174:

Here's an example on how to add a tracing span to a function:

func foo(ctx buffalo.Context) {
    sp, ctx := buffet.ChildSpan("storage.save", ctx)
    defer sp.Close()
    // do stuff with ctx
}

---

I'll start with List and Save.

[manugupt1]

Hi @tbpg, I had like to take a look at this issue!

[tbpg]

buffet works with buffalo.Context for requests. So, we'll need to use the tracing library directly to add the spans. Should we use opencensus or opentracing? buffet seems to use opentracing, so should we continue with that?

Here's how to create an opentracing span:

span, ctx := opentracing.StartSpanFromContext(ctx, "fs.List")
defer span.Finish()

Here's how to create an opencensus span:

ctx, span := trace.StartSpan(ctx, "cache.Get")
defer span.End()

The APIs differ in how to create tracers, tags, & metrics, but I haven't used either extensively. If there hasn't been explicit decision, it might be good to split the tracing library question into a separate issue.

[arschles]

Please do!

On Tue, Jul 24, 2018 at 07:18 Manu Gupta [e-mail] wrote:

Hi @tbpg https://github.com/tbpg, I had like to take a look at this
issue!

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/gomods/athens/issues/324#issuecomment-407386325, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEU0UQRj6UWqV3JD51VsXbrhVYEuPzYks5uJxCwgaJpZM4VcnI9
.

[robjloranger]

:+1: for using opentracing until we fully convert to threading the buffalo context.

[manugupt1]

Got started on it here: https://github.com/gomods/athens/pull/326

The storage.Get function touches a lot of other backends as well as the context was not defined earlier, so I am going to hold it off for a different commit.

[arschles]

@robjloranger is there an issue for fully convert to threading the buffalo context? If not, after #326 is merged, we should start talking about that in here

[robjloranger]

174

[manugupt1]

@tbpg Just finished all of the items on the check list.
I have added a few more in this PR: https://github.com/gomods/athens/pull/332

Would you like me to just add for everything else in the storage package like GCP?

[manugupt1]

There is another one remaining which is the olympus backend?

[manugupt1]

Ref: https://github.com/gomods/athens/issues/392

[marpio]

@manugupt1 @tbpg Can we close this issue?

[manugupt1]

@marpio No :( There is a bug in the tracing thing. Right now, I am reading up on integrating about opencensus since we decided to use opencensus.

I am on vacation this Friday - Monday, I am planning to write code after that. If anyone is willing to do it before that, it will be great too.

[manugupt1]

@arschles @marpio This issue was fixed with #604