Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

Custom events cannot render in Appdash (cannot call RegisterEvent) #67

Open
slimsag opened this issue May 15, 2015 · 5 comments
Open

Custom events cannot render in Appdash (cannot call RegisterEvent) #67

slimsag opened this issue May 15, 2015 · 5 comments

Comments

@slimsag
Copy link
Member

slimsag commented May 15, 2015

When running appdash on the command line, custom events don't render. This is because nobody in the entire application can invoke RegisterEvent with the new event type.

I have a few ideas on how we can fix this.

As a fast-and-easy hack for now: you can copy the source for cmd/appdash (or just modify it directly) such that a call to RegisterEvent for your type is made.

Reported by @joeshaw over Slack

Here is what running cmd/appdash should look like:


image

image

And here is what it ends up looking like (because the custom event type is not registered):


image

image

@slimsag slimsag self-assigned this May 15, 2015
@slimsag
Copy link
Member Author

slimsag commented May 19, 2015

The root of this issue seems to be the annotation serialization model that we've chosen.

  • To marshal or unmarshal an event, it must be registered.
  • Registration must occur because there are various interfaces (TimespanEvent, TimestampedEvent, ImportantEvent) which direct information about the event type (e.g. for TimespanEvent it tells us which struct field exactly is the start and end times of the span).
  • Without registration we cannot know the timespan times, important fields, etc.

The stringent requirement on event registration means that custom event types don't render properly when running the standalone appdash command (as shown above).

We could formalize the annotation (read: struct field) name for both the starting and ending times of a timespan (or the timestamp for a timestamped event) but this would effectively render the TimespanEvent interface a bit redundant (and users of it would soon find their events not rendering in the standalone appdash app).

The least imposing option I can think of is to keep the redundant TimespanEvent interface for legacy code, and encourage the use of a new type Timespan struct{StartTime, EndTime time.Time} structure (which would always have identical StartTime and EndTime annotation names, so the standalone app could always unmarshal at least a Timespan event for rendering).

@joeshaw
Copy link
Contributor

joeshaw commented May 19, 2015

The idea I had, without any idea of how possible or difficult it would be to implement: Tie the registration of the event to a collector, and send a "schema serialization" to the collector to inform it how a particularly named event is structured.

In our app we currently call appdash.RegisterEvent() in init(), but there's no reason why we couldn't call it after we create a collector.

@slimsag
Copy link
Member Author

slimsag commented May 22, 2015

@joeshaw Right, I think your solution would work. It does require a change to the core annotation serialization model, however.

I'm not opposed to changing the annotation serialization model, and it's required in order to fix this, but it does need some good thought on what the best approach is (how does Zipkin/Dapper handle it? what other approaches are there? etc), I just want to ensure the solution is not rushed 😄

For now I think the best approach is to require modification of the appdash executable's source code, which should be as simple as:

import "my/event/declarations"

(At least until I'm certain of a proper solution to this issue).

@joeshaw
Copy link
Contributor

joeshaw commented May 22, 2015

@slimsag yep, not trying to rush you on this. We've worked around this in the meantime exactly as you suggest. Thanks!

@slimsag
Copy link
Member Author

slimsag commented Apr 25, 2016

The proper solution here is probably the opentracing API which does not use custom event types but rather standard methods for adding tags/etc to spans.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants