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

Enhancement/#1312 add in distributed tracing #1313

Open
wants to merge 2 commits into
base: release/3.2
Choose a base branch
from

Conversation

thompson-tomo
Copy link
Contributor

Description

To improve traceability activities are now being created so that external requests can be traced

Closes #1312

Quality checklist

  • Your code complies with our Coding Style.
  • You've updated documentation for your change, where applicable.
    If your change affects other repositories, such as Documentation, Samples and/or MainSite, add linked PRs here.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

@bart-vmware
Copy link
Member

I don't see the point of adding this, am I missing something? ASP.NET already provides built-in tracing for HttpClient usage.

@thompson-tomo
Copy link
Contributor Author

thompson-tomo commented Jun 11, 2024

Without this change the log messages being generated by steeltoe do not have the tracing properties (trace, transaction & span) hence I was ending up with alot of messages missing the tracing properties.

Note most of the changes are whitespace due to introduction of the using statements.

@TimHess
Copy link
Member

TimHess commented Jun 11, 2024

How are tracing properties useful on these items?

These interactions are typically called in the background rather than as a part of any distributed interaction.

@thompson-tomo
Copy link
Contributor Author

So what i was noticing is that for example the background requests to eureka server, i was not able to correlate the logs being generated prior to the http requests with the server processing of the request and at the same time was not able to connect the processing of the response to the original request.

@bart-vmware
Copy link
Member

I don't understand why a periodic background request to eureka should be correlated to an incoming request. There's no relationship between those execution flows. For example:

18:43:01 Incoming HTTP request at /api/shoppingbasket, correlation ID matching click in UI
18:43:05 Incoming HTTP request at /api/shoppingbasket, correlation ID matching click in UI
18:43:08 Incoming HTTP request at /api/shoppingbasket, correlation ID matching click in UI
18:43:09 Outgoing eureka request, no correlation
18:43:12 Incoming HTTP request at /api/shoppingbasket, correlation ID matching click in UI

@thompson-tomo
Copy link
Contributor Author

thompson-tomo commented Jun 14, 2024

The reason for me is so that the logs generated prior to the transmission ie

18:43:09 Outgoing eureka request

Can be correlated through the logs as the request traverses the environment ie via a reverse proxy/load balancer & then ideally your eureka/consul server etc. At the same time there was some endpoints which weren't generating activities.

@thompson-tomo thompson-tomo force-pushed the enhancement/#1312_AddInDistributedTracing branch from ab06346 to 444e26c Compare July 18, 2024 22:08
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.

3 participants