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

Fetching records by ID should be time agnostic #19

Open
JBAhire opened this issue Aug 21, 2020 · 5 comments
Open

Fetching records by ID should be time agnostic #19

JBAhire opened this issue Aug 21, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request refactor

Comments

@JBAhire
Copy link
Member

JBAhire commented Aug 21, 2020

Fetching records by ID is currently time range dependent. We observed that time range is essential for each query we do. But in case I am searching for trace using Trace ID which is unique parameter it's going to return only one trace but if that trace is not there in that time range, I won't get that back.

ex: If I am searching for Trace with ID b210fcf03d5eb036.

  • The below query won't return result because of time range.
{
  traces(
    type: API_TRACE
    between: {
      startTime: "2020-08-22T06:43:05.358Z"
      endTime: "2020-08-22T08:43:05.358Z"
    }
    filterBy: [
      {
        operator: EQUALS
        value: "b210fcf03d5eb036"
        type: ID
        idType: API_TRACE
      }
    ]
  ) {
    results {
      id
    }
  }
}

Result:

{
  "data": {
    "traces": {
      "results": []
    }
  }
}
  • But the same query without time range will work fine and give us trace.
{
  traces(
    type: API_TRACE
    filterBy: [
      {
        operator: EQUALS
        value: "b210fcf03d5eb036"
        type: ID
        idType: API_TRACE
      }
    ]
  ) {
    results {
      id
    }
  }
}

Why is this helpful?

  • go to trace feature: common in other tracing systems like zipkin, as logs have trace IDs so pasting one into the UI
    • not sure this would be in hypertrace UI or hypertrace core UI
  • end-to-end smoke test: also common is the ability to read back via api, what you've written, as HT has two api layers (gRPC and GraphQL) we should execute this in GraphQL
    • that's this issue, which might also belong in hypertrace core graphql not sure
  • reduce perceived complexity. there are a number of complexity issues, some caused by tech choices, some our choices within them. Being able to show the simplest api simply is also rationale. For example, with a simpler query, users can fetch a trace by ID without POST body syntax.

cc: @adriancole @skjindal93

@codefromthecrypt
Copy link
Contributor

yep I don't think any of the tracing systems would be able to get by without a trace ID index. There may be some practical limitations, such as how far back to look for a trace ID and not since 1970, but right now Zipkin etc wouldn't require a range query to get a trace by ID.

@aaron-steinfeld
Copy link
Contributor

We've discussed prior and do need this API, but as planned it should simplify down to:

{
  trace(id: "b210fcf03d5eb036") { // TBD about type param
      id
  }
}

Right now, the HT UI doesn't need this so it hasn't been a priority. It will require query layer changes before we can support this in graphql though.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 22, 2020 via email

@aaron-steinfeld
Copy link
Contributor

It would definitely be feasible to implement this in graphql with a dummy time range on the internal query, just gets to be a question of relative priority (is it important to get this out in some form immediately, vs spending more time to wait for a better implementation), and whether there are performance implications to that type of implementation (I don't know, but I would suspect yes).

@codefromthecrypt
Copy link
Contributor

I added rationale to the description. I think for now, we can try the dummy range to allow smoke tests and "simplest api" instructions to operate without having to know up front the time range the trace is in. Meanwhile, the internal implementation probably should go forward when resources allow. Otherwise, we should mark this help-wanted if we don't have resources, so someone else can do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor
Projects
None yet
Development

No branches or pull requests

5 participants