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.

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

Add Lazy attributes for Events #384

Closed
GoPavel opened this issue Jan 28, 2020 · 3 comments · Fixed by #474
Closed

Add Lazy attributes for Events #384

GoPavel opened this issue Jan 28, 2020 · 3 comments · Fixed by #474
Assignees
Labels

Comments

@GoPavel
Copy link
Contributor

GoPavel commented Jan 28, 2020

Specification of Event states that Lazy Event is

lazily constructed, with the intention of avoiding unnecessary work if an event is unused

But now it's not true. In resolved PR#130 mentions lazy attributes for Event and Link in the future, but I don't see an issue about it.

Current code of Span.add_lazy_event and Span.add_event in SDK looks strange.

@mauriciovasquezbernal
Copy link
Member

Hi @GoPavel,

You're right, the current implementation is wrong and it doesn't support lazy events.
A possible implementation could be to add Callable as an attribute type, exporters should perform a type check and invoke the attribute if it is a Callable to get its value.

@Oberon00
Copy link
Member

Oberon00 commented Feb 4, 2020

Does the same apply to "lazy" links? That also always looked strange to me but I never really understood the purpose of lazy links tbh.

@mauriciovasquezbernal
Copy link
Member

Yes, lazy links were also wrongly implemented by me, somehow I misunderstood that lazy meant to receive an Event object instead of receiving the parameters and constructing the object internally.

Btw, lazy links don't exist anymore #259.

I think we can provide real support for lazy events using Callable in Python.

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

Successfully merging a pull request may close this issue.

3 participants