Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Tracing #22

Open
wants to merge 47 commits into
base: cernbox
Choose a base branch
from
Open

Tracing #22

wants to merge 47 commits into from

Conversation

vascoguita
Copy link

No description provided.

@labkode
Copy link
Member

labkode commented Sep 2, 2022

@gmgigi96 @glpatcern this may look it's a big PR with many crucial changes but is not. Most changes are just adding the instrumentation to the code, modulo there is some refactoring of the tracing logic.

I would like to test this PR in the QA environment to start getting some load to find perf limits, however I prefer to follow the typical release cycle to merge this and generate a new version that will be deployed only on QA.

I went with Vasco through the code and looks good, the configuration is already in Puppet.

Please do a review as well and then we can tag.

@labkode labkode requested review from gmgigi96 and removed request for ishank011 September 2, 2022 13:55
Copy link
Member

@glpatcern glpatcern left a comment

Choose a reason for hiding this comment

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

I think it's fine to go ahead for QA field tests. My only comment would be on the style of coding the SpanStart calls spread over the whole code base, which all look like:

r, span := tracing.SpanStartFromRequest(r, tracerName, "<name of the function>")

or

ctx, span := tracing.SpanStartFromContext(ctx, tracerName, "<name of the function>")

where the name of the function is hard coded each time. If some reflection logic could be used, that could be worked out from the calling stack instead, but I'm not sure this is at all possible in Go.

… ocmshareprovider, permissions, preferences, publicshareprovider, helloworld, publicstorageprovider, userprovider and usershareprovider GRPC services
…n the request and refactor otelhttp handler application
… helloworld, mentix, meshdirectory, metrics, ocmd, preferences, prometheus, reverseprocy, siteacc, sysinfo and welknown HTTP service with opentelemetry
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