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

Add additional tracing fields specific to Scylla #816

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

karlpvoss
Copy link

Introduction of some new struct fields that express the additional fields that are present in the
Scylla schemas for system_traces.sessions and system_traces.events.

Since I was unable to write a FromRow implementation that I was happy with in any form, the
provided implementation will perform conversion for only the fields present in both databases.
This therefore also stands for Session::get_tracing_info, where the documentation has been
updated.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
    If there's any feedback about what kind of tests should be added for this type of change let me know and I'll do them.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@piodul
Copy link
Collaborator

piodul commented Oct 13, 2023

Hi @karlpvoss, could you tell me about the use case for this PR? As you wrote, this PR adds fields to the tracing-related structs, but never initializes them. Was that the intention, or do you need tips on how to fill these structs in case when connected to Scylla? If it's the former, then I'd rather not add fields that are unused - it's slightly confusing as users who won't carefully read the docstring for get_tracing_info or the docs.

Implementing it right now might be a bit convoluted, but doable, as you would need to first detect whether you are connected to Scylla or Cassandra and then use a query that fetches the columns supported by either Scylla or Cassandra. With better deserialization traits (#462), you could SELECT * from the tables and then fill in the struct in the deserialization trait as you would get information about the names of the columns, though it's difficult for me to tell when the new deserialization traits will be implemented and merged.

@Lorak-mmk Lorak-mmk added the waiting-on-author Waiting for a response from issue/PR author label Mar 14, 2024
@wprzytula
Copy link
Collaborator

@karlpvoss what's the status of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author Waiting for a response from issue/PR author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants