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

SOLR-16935 Add spans to cover gaps in query processing #1853

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

stillalex
Copy link
Member

@stillalex stillalex commented Aug 18, 2023

https://issues.apache.org/jira/browse/SOLR-16935

Description

This is a followup of #1841 where I am proposing 2 new spans for query processing, to close some of the unknown gaps in the duration.

Will do a complete review of all spans we are emitting too.

Solution

Please provide a short description of the approach taken to implement your solution.

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@stillalex stillalex requested a review from janhoy August 18, 2023 13:17
@stillalex
Copy link
Member Author

stillalex commented Aug 18, 2023

Span Data Review

All requests currently have the root span:

Name: name that reflect incoming request (operation, url as template)
                                (example) get:/{collection}/select
                                (example) post:/{collection}/update

Tags
db.instance	test    -- collection or core name (ex. test OR test_shard2_replica_n2)
http.method	GET
http.request.method	GET
http.params	_=1692378454124&indent=true&q=*:*&q.op=OR&useParams=
http.response.status_code	200
http.status_code	200
http.url	http://localhost:8983/solr/test/select

db.type	 solr    -- fixed value, present on root spans only

internal.span.format	proto    -- fixed value, present on all spans
otel.library.name	solr        -- fixed value, present on all spans
span.kind	server            -- fixed value, present on all spans

New child spans covering more of the request processing, added with this PR. these spans are present on all requests not just Query

Name: CollectionsHandler     -- dynamic name,  the handler class name that is used for processing this request
Tags
  internal.span.format	proto    -- fixed value, present on all spans
  otel.library.name	solr    -- fixed value, present on all spans
  span.kind	internal    -- fixed value, present on all spans
Name: JacksonJsonWriter -- dynamic name,  the class name
Tags
  contentType	application/json; charset=UTF-8
  internal.span.format	proto    -- fixed value, present on all spans
  otel.library.name	solr    -- fixed value, present on all spans
  span.kind	internal    -- fixed value, present on all spans

@janhoy this is a list of tags we export. I am thinking it might be a good idea to add it to the ref guide?

@stillalex
Copy link
Member Author

stillalex commented Aug 18, 2023

this is how this looks after adding the 2 new spans. when we look at the internal communication, there are still some gaps. maybe it would be interesting to fill that in too.
[edit] nvm I think this is just network comm between the nodes, will leave it as is for now.

Screenshot 2023-08-18 at 10 53 00 AM

@stillalex
Copy link
Member Author

another example of gaps
Screenshot 2023-08-18 at 10 55 38 AM

@stillalex stillalex marked this pull request as draft August 31, 2023 19:12
@stillalex
Copy link
Member Author

converted to draft so no one accidentally merges this. the PR is waiting review from @dsmiley and potentially some contributions, as requested on the Jira ticket.

@stillalex stillalex force-pushed the SOLR-16935-otel-newspans branch 2 times, most recently from b269860 to 90ab034 Compare September 27, 2023 15:00
@stillalex stillalex marked this pull request as ready for review September 27, 2023 15:02
Copy link

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the dev@solr.apache.org mailing list. Thank you for your contribution!

@github-actions github-actions bot added the stale PR not updated in 60 days label Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR not updated in 60 days
Projects
None yet
3 participants