Do not skip service/operation indexing for firehose spans + a couple fixes#2090
Conversation
| return nil | ||
| } | ||
|
|
||
| func (s *SpanWriter) writeOtherIndexes(span *model.Span, ds *dbmodel.Span) error { |
There was a problem hiding this comment.
Could you use a more descriptive name here?
There was a problem hiding this comment.
Perhaps this could just be writeIndexes like earlier, but it calls writeServiceOperationIndex? Might simplify the branching in writer.go as well.
| assert.Equal(t, "planet-express", serviceWritten.Load()) | ||
| assert.Equal(t, dbmodel.Operation{ | ||
| ServiceName: "planet-express", | ||
| SpanKind: "", | ||
| OperationName: "package-delivery", | ||
| }, operationWritten.Load()) |
There was a problem hiding this comment.
Are these not tested elsewhere? For e.g., if plugin/storage/cassandra/spanstore/writer.go L183 were removed - wouldn't other tests fail?
There was a problem hiding this comment.
I am not testing whether index writing works, but whether it's invoked for firehose span. This is the only test that makes distinction between two types of indices.
Codecov Report
@@ Coverage Diff @@
## master #2090 +/- ##
=========================================
+ Coverage 96.29% 96.3% +<.01%
=========================================
Files 214 214
Lines 10540 10542 +2
=========================================
+ Hits 10150 10152 +2
+ Misses 331 330 -1
- Partials 59 60 +1
Continue to review full report at Codecov.
|
Signed-off-by: Yuri Shkuro <ys@uber.com>
yurishkuro
force-pushed
the
dont-skip-indexing-ops
branch
from
49f08d1 to
19369d0
Compare
Signed-off-by: Yuri Shkuro <ys@uber.com>
…ymore. which cqlsh prints /opt/cassandra/bin/cqlsh Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
| } | ||
| tr.mb.AssertGaugeMetrics(t, | ||
| metricstest.ExpectedMetric{Name: "client_stats.connected_clients", Value: test.expGauge}) | ||
| assert.EqualValues(t, test.expGauge, gaugeValue) |
There was a problem hiding this comment.
Unrelated change to fix a flaky test. There was a race condition with gauge value being 1 inside the loop and then dropping back to 0 by the time the AssertGaugeMetrics was called.
| # that allows installing Jaeger keyspace and schema without installing cqlsh. | ||
|
|
||
| CQLSH=${CQLSH:-"/usr/bin/cqlsh"} | ||
| CQLSH=${CQLSH:-"/opt/cassandra/bin/cqlsh"} |
There was a problem hiding this comment.
unrelated change to fix regression in Cassandra tests that were breaking CI.
yurishkuro
changed the title
2 participants
Which problem is this PR solving?
The transitive dep graphs view uses service & operation index to populate the dropdown. However, when traces are stored in the firehose mode, they are excluded from indexing (to reduce the load on storage), including the service/operation tables, which are actually very inexpensive to write because writes only happen once per 12hr period due to a cache in front of them.
Short description of the changes
Allow service/operation tables to be written even in firehose mode.