Skip to content

Support Tracing for Interservice gRPC calls #1394

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

Conversation

coolwednesday
Copy link
Contributor

@coolwednesday coolwednesday commented Jan 16, 2025

Description:

Checklist:

  • I have formatted my code using goimport and golangci-lint.
  • All new code is covered by unit tests.
  • This PR does not decrease the overall code coverage.
  • I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

Copy link
Member

@aryanmehrotra aryanmehrotra left a comment

Choose a reason for hiding this comment

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

  1. Readme can be update either to refer the docs at gofr.dev/docs such that user can understand how these files are created.

  2. Can we structure the examples in a better way -
    For example, for interservice http communication we dont have 2 examples.
    one way could be to have grpc and inside that we have two separate directories for a client and a server

@aryanmehrotra
Copy link
Member

aryanmehrotra commented Jan 18, 2025

Logs needs to be fixed - BADWIDTH
Screenshot 2025-01-19 at 1 56 46 AM

Metadata should also contain the correaltionID - can be part of different issue ticket
Screenshot 2025-01-19 at 1 59 31 AM

When an external request is made - we should also log it similar to interservice http calls - can be part of a different ticket
Screenshot 2025-01-19 at 2 02 24 AM

@coolwednesday
Copy link
Contributor Author

Logs needs to be fixed - BADWIDTH Screenshot 2025-01-19 at 1 56 46 AM

Metadata should also contain the correaltionID - can be part of different issue ticket Screenshot 2025-01-19 at 1 59 31 AM

When an external request is made - we should also log it similar to interservice http calls - can be part of a different ticket Screenshot 2025-01-19 at 2 02 24 AM

In my implementation, I am only adding correlation ID that is the traceID, when the user actually initiates a call using gofr-CLI's generated gRPC Client. So only incoming context, while sending request carries metadata and none of the corresponding response/outgoing context include correlation ID in the metadata.

So, is it necessary in any case to have metadata with correlation ID in the outgoing context received ?

mdsf is a tool that formats Markdown files with fenced code blocks.
https://github.com/hougesen/mdsf

It is used to ensure that all code blocks are formatted in a consistent way.
There are no such a thing as an indented fenced code block in Markdown.

This commit fixes the indentation of fenced code blocks.

Files are then formatted with mdsf tool.
@ccoVeille
Copy link
Contributor

I'm a bit surprised to see my recent commits I made in this PR.

It looks like to me you had to fight conflicts at some points. You apparently sorted them. 👍

Screenshot_20250120_084420_GitHub.jpg

I'm used to rebase-only workflow. Maybe it's OK in a merge-workflow 🤔 🤷

But I prefer to report it

@coolwednesday
Copy link
Contributor Author

As there are multiple changes in files ... that are not related to the PR itself. I will be closing the PR and creating a new one.

Link to the new PR : #1408

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

Successfully merging this pull request may close these issues.

5 participants