Add support to the server.Queryer to query against a service's name. #27
Conversation
|
It's worth noting that this may have some unexpected results. Namely, if A-svc calls B-svc, and you're expecting a search for B-svc to show that call from A-svc, this will not be the case. This can probably be done with a better Postgres query, however calls to B-svc will show up if you search A-svc. I'm not familiar with zipkin at all, so I'm not sure if this is proper behaviour, just something I felt I should note for completeness. |
|
I think it should return all traces that involve the service, not just traces that originate from the service. |
|
I'll be honest, I'm not entirely sure how that could be done, my SQL-fu isn't that good. |
|
@dominikh did some stabbing around with my live setup, the latest commit appears to be doing the job. Only issue is now there's an issue with the generated protos being out of date for the latest grpc, but I don't see a way to generate the files locally, and I've got my own custom arcane voodoo for generation myself. |
| @@ -378,6 +385,7 @@ WHERE | |||
| (? = '' OR operation_name = ?) AND | |||
| DURATION(time) >= ? AND | |||
| DURATION(time) <= ? AND | |||
| EXISTS ( SELECT 1 FROM spans AS sub_spans WHERE sub_spans.trace_id = spans.trace_id AND sub_spans.service_name IN (` + strings.Join(serviceConds, ", ") + `)) AND | |||
There was a problem hiding this comment.
What happens if no ServiceNames were provided and serviceConds is empty? Is an empty IN valid and does it match?
There was a problem hiding this comment.
The zipkin ui does not appear to allow searching with no service name, so I feel this is a moot point for now.
There was a problem hiding this comment.
The API allows it; and even if the contract didn't, we do not want to generate invalid SQL on a possible code path. We should either support it, or detect and reject it.
The PostgreSQL documentation documents IN as expression IN (value [, ...]), which would suggest that an empty IN is not valid.
| @@ -137,6 +137,8 @@ type Query struct { | |||
| OrTags []QueryTag | |||
| // How many traces to return. Zero means no limit. | |||
| Num int | |||
| // ServiceName to filter by. | |||
There was a problem hiding this comment.
Nitpick, but did you mean to write "// ServiceNames to filter by." here? Since the variable is called ServiceNames. Or was the removal of "s" intentional?
This fixes tracer#26 as well.
If you gave any of query.AndTags or query.OrTags -- the resulting query was invalid, as it had an extra trailing ')'
No description provided.