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

feat(handler) remove dependency on base plugin #50

Merged
merged 2 commits into from
Oct 9, 2019
Merged

Conversation

kikito
Copy link
Member

@kikito kikito commented Oct 8, 2019

This PR also re-indents all the source code with spaces.


-- We want to run first so that timestamps taken are at start of the phase
-- also so that other plugins might be able to use our structures
OpenTracingHandler.PRIORITY = 100000

function OpenTracingHandler:new(name)
Copy link

@hishamhm hishamhm Oct 8, 2019

Choose a reason for hiding this comment

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

Is this :new function still being used with the removal of BasePlugin?

In other plugins where BasePlugin was removed, the :new operation was removed altogether. Generally, it was just boilerplate, but in the case of this plugin it does a thing in line 23 below which I'm not seeing in Kong's plugin handling how it ever gets executed (though I may be just missing it).

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, I missed the new function. Removing it.


-- We want to run first so that timestamps taken are at start of the phase
-- also so that other plugins might be able to use our structures
OpenTracingHandler.PRIORITY = 100000

function OpenTracingHandler:new(name)
OpenTracingHandler.super.new(self, name or "opentracing")
Copy link

Choose a reason for hiding this comment

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

super is being set to self below... Wouldn't this cause an infinite loop if this line was ever to run? (Seems to indicate :new is never being called, in which case self.conf_to_tracer needs to be initialized somewhere else)

Copy link
Member Author

Choose a reason for hiding this comment

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

infinite loop

Removed :new, so that won't happen here.

self.conf_to_tracer

That member variable seemed to be a simple "cache" for traces. I made it a "module local cache" instead.

@hishamhm hishamhm merged commit 5d5c1bb into master Oct 9, 2019
@hishamhm hishamhm deleted the feat/remove-base branch October 9, 2019 16:40
kikito pushed a commit that referenced this pull request Oct 14, 2019
kikito pushed a commit that referenced this pull request Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants