Skip to content

Commit ee197d1

Browse files
liran-funaroliran-funaro
committed
Address PR comments
Signed-off-by: Liran Funaro <liran.funaro@gmail.com>
1 parent eef9b80 commit ee197d1

5 files changed

Lines changed: 89 additions & 18 deletions

File tree

‎docs/metrics_reference.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ The following Sidecar metrics are exported for consumption by Prometheus.
4141
| sidecar_delivery_stream_errors_total | counter | stream_type source_id error_type | Total number of stream errors by stream type, source ID, and error type. |
4242
| sidecar_delivery_withholding_suspicion_raised_total | counter | source_id | Total number of block withholding suspicions raised by data source ID. |
4343
| sidecar_delivery_withholding_suspicion_cleared_total | counter | source_id | Total number of block withholding suspicions cleared by data source ID. |
44+
| sidecar_delivery_withholding_suspicion_confirmed_total | counter | source_id | Total number of block withholding suspicions confirmed by data source ID. |
4445
| sidecar_delivery_stream_block_gap | gauge | source_id | Current block gap between header and data streams (data - header) by data source ID.Positive value indicates that data stream is ahead of header stream. |
45-
| sidecar_delivery_target_arrival_deadline | gauge | source_id | Unix timestamp of the target block arrival deadline for withholding detection by source ID. |
46+
| sidecar_delivery_target_arrival_deadline_unix_milliseconds | gauge | source_id | Unix timestamp (milliseconds) of the target block arrival deadline for withholding detection by source ID. |
4647
| sidecar_delivery_config_updates_total | counter | | Total number of config block updates detected. |
4748
| sidecar_delivery_config_block_number | gauge | | Current config block number. |
4849

‎mock/test_exports.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,10 @@ func (e *OrdererTestEnv) WaitForBlock(t *testing.T, outputBlocks <-chan *common.
350350
b, ok := channel.NewReader(t.Context(), outputBlocks).ReadWithTimeout(3 * time.Second)
351351
require.True(t, ok, "failed to receive block from output channel")
352352
require.NotNil(t, b)
353+
t.Logf("Received block #%d", b.Header.Number)
353354
if e.PrevBlock != nil {
354355
require.Equal(t, e.PrevBlock.Header.Number+1, b.Header.Number)
355356
}
356-
t.Logf("Received block #%d", b.Header.Number)
357357
e.PrevBlock = b
358358
return b
359359
}

‎utils/deliverorderer/metrics.go‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
type Metrics struct {
1919
// Stream Progress Metrics.
2020
CurrentDataSourceID prometheus.Gauge
21-
StreamBlockNumber prometheus.GaugeVec // labels: stream_type
21+
StreamBlockNumber *prometheus.GaugeVec // labels: stream_type
2222
BlocksDeliveredTotal *prometheus.CounterVec // labels: stream_type, source_id
2323

2424
// Block Verification Metrics.
@@ -31,10 +31,11 @@ type Metrics struct {
3131
StreamErrorsTotal *prometheus.CounterVec // labels: stream_type, source_id, error_type
3232

3333
// Block Withholding Detection (BFT mode).
34-
BlockGapGauge *prometheus.GaugeVec // labels: source_id
35-
SuspicionRaisedTotal *prometheus.CounterVec // labels: source_id
36-
SuspicionClearedTotal *prometheus.CounterVec // labels: source_id
37-
TargetArrivalDeadline *prometheus.GaugeVec // labels: source_id
34+
BlockGapGauge *prometheus.GaugeVec // labels: source_id
35+
SuspicionRaisedTotal *prometheus.CounterVec // labels: source_id
36+
SuspicionClearedTotal *prometheus.CounterVec // labels: source_id
37+
SuspicionConfirmedTotal *prometheus.CounterVec // labels: source_id
38+
TargetArrivalDeadline *prometheus.GaugeVec // labels: source_id
3839

3940
// Config Updates.
4041
ConfigUpdatesTotal prometheus.Counter
@@ -52,7 +53,7 @@ func NewMetrics(p *monitoring.Provider, params monitoring.MetricsParameters) *Me
5253
Name: "data_source_id",
5354
Help: "Current orderer party ID used as the data block source.",
5455
}),
55-
StreamBlockNumber: *p.NewGaugeVec(prometheus.GaugeOpts{
56+
StreamBlockNumber: p.NewGaugeVec(prometheus.GaugeOpts{
5657
Namespace: params.Namespace,
5758
Subsystem: params.Subsystem,
5859
Name: "block_number",
@@ -113,6 +114,12 @@ func NewMetrics(p *monitoring.Provider, params monitoring.MetricsParameters) *Me
113114
Name: "withholding_suspicion_cleared_total",
114115
Help: "Total number of block withholding suspicions cleared by data source ID.",
115116
}, []string{"source_id"}),
117+
SuspicionConfirmedTotal: p.NewCounterVec(prometheus.CounterOpts{
118+
Namespace: params.Namespace,
119+
Subsystem: params.Subsystem,
120+
Name: "withholding_suspicion_confirmed_total",
121+
Help: "Total number of block withholding suspicions confirmed by data source ID.",
122+
}, []string{"source_id"}),
116123
BlockGapGauge: p.NewGaugeVec(prometheus.GaugeOpts{
117124
Namespace: params.Namespace,
118125
Subsystem: params.Subsystem,
@@ -123,8 +130,9 @@ func NewMetrics(p *monitoring.Provider, params monitoring.MetricsParameters) *Me
123130
TargetArrivalDeadline: p.NewGaugeVec(prometheus.GaugeOpts{
124131
Namespace: params.Namespace,
125132
Subsystem: params.Subsystem,
126-
Name: "target_arrival_deadline",
127-
Help: "Unix timestamp of the target block arrival deadline for withholding detection by source ID.",
133+
Name: "target_arrival_deadline_unix_milliseconds",
134+
Help: "Unix timestamp (milliseconds) of the target block arrival deadline for withholding " +
135+
"detection by source ID.",
128136
}, []string{"source_id"}),
129137

130138
// Config Updates

‎utils/deliverorderer/orderer.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ func (d *ftDelivery) checkBlockWithholding() error {
316316
// Clear suspicion if data stream caught up.
317317
if d.targetNextBlockNum > 0 {
318318
d.metrics.SuspicionClearedTotal.WithLabelValues(dataSourceIDLabel).Inc()
319+
d.metrics.TargetArrivalDeadline.WithLabelValues(dataSourceIDLabel).Set(0)
319320
d.targetNextBlockNum = 0
320321
}
321322
return nil
@@ -327,9 +328,9 @@ func (d *ftDelivery) checkBlockWithholding() error {
327328
//nolint:gosec // Capping the gap at [math.MaxInt64].
328329
gapDuration := time.Duration(min(d.targetNextBlockNum-d.dataStream.nextBlockNum, math.MaxInt64))
329330
d.targetArrivalTime = time.Now().Add(d.params.SuspicionGracePeriodPerBlock * gapDuration)
330-
targetArrivalTimeUnix := float64(d.targetArrivalTime.Unix())
331-
d.metrics.TargetArrivalDeadline.WithLabelValues(dataSourceIDLabel).Set(targetArrivalTimeUnix)
331+
targetArrivalTimeUnix := float64(d.targetArrivalTime.UnixMilli())
332332
d.metrics.SuspicionRaisedTotal.WithLabelValues(dataSourceIDLabel).Inc()
333+
d.metrics.TargetArrivalDeadline.WithLabelValues(dataSourceIDLabel).Set(targetArrivalTimeUnix)
333334
return nil
334335
}
335336

@@ -344,7 +345,9 @@ func (d *ftDelivery) checkBlockWithholding() error {
344345
d.dataStream.nextBlockNum, d.curDataBlockSourceID, d.headerOnlyStream.updaterSourceID,
345346
d.headerOnlyStream.nextBlockNum-1)
346347

347-
// Reset the target block (clears the suspicion).
348+
d.metrics.SuspicionConfirmedTotal.WithLabelValues(dataSourceIDLabel).Inc()
349+
d.metrics.TargetArrivalDeadline.WithLabelValues(dataSourceIDLabel).Set(0)
350+
// Reset the target block (clears the suspicion for the next source).
348351
d.targetNextBlockNum = 0
349352
return errSuspicion
350353
}

‎utils/deliverorderer/orderer_test.go‎

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111
"os"
1212
"path/filepath"
13+
"strconv"
1314
"sync"
1415
"testing"
1516
"time"
@@ -415,10 +416,8 @@ func TestOrdererDeliverBFT(t *testing.T) {
415416
// Verify suspicion metrics for party 2
416417
suspicionCount2 := test.GetIntMetricValue(t, m.SuspicionRaisedTotal.WithLabelValues("2"))
417418
require.GreaterOrEqual(t, suspicionCount2, 1, "suspicion should be raised for party 2")
418-
419-
// Verify target arrival deadline was set when suspicion was raised
420-
targetDeadline := test.GetMetricValue(t, m.TargetArrivalDeadline.WithLabelValues("2"))
421-
require.Greater(t, targetDeadline, float64(0), "target arrival deadline should be set")
419+
suspicionConfirmCount2 := test.GetIntMetricValue(t, m.SuspicionConfirmedTotal.WithLabelValues("2"))
420+
require.GreaterOrEqual(t, suspicionConfirmCount2, 1, "suspicion should be confirmed for party 2")
422421

423422
currentSource = test.GetIntMetricValue(t, m.CurrentDataSourceID)
424423
require.Equal(t, 1, currentSource, "current data source should be party 1")
@@ -478,7 +477,7 @@ func TestOrdererDeliverBFT(t *testing.T) {
478477
stopDelivery()
479478

480479
p1.HoldFromBlock.Store(0)
481-
e.startDelivery(t)
480+
stopDelivery = e.startDelivery(t)
482481
b = e.submit(t)
483482
require.EqualValues(t, 1, b.SourceID)
484483

@@ -499,6 +498,66 @@ func TestOrdererDeliverBFT(t *testing.T) {
499498

500499
finalBlockNum := test.GetIntMetricValue(t, m.StreamBlockNumber.WithLabelValues("data"))
501500
require.GreaterOrEqual(t, finalBlockNum, 5, "should have delivered multiple blocks")
501+
502+
t.Log("Restart stream with longer suspecion deadline")
503+
stopDelivery()
504+
p0.HoldFromBlock.Store(0)
505+
p0.ReplaceBlock.Clear()
506+
p1.HoldFromBlock.Store(0)
507+
p1.ReplaceBlock.Clear()
508+
p2.HoldFromBlock.Store(0)
509+
p2.ReplaceBlock.Clear()
510+
e.deliveryParams.SuspicionGracePeriodPerBlock = time.Hour
511+
512+
stopDelivery = e.startDelivery(t)
513+
b = e.submit(t)
514+
curSource := b.SourceID
515+
curSourceLabel := strconv.FormatUint(uint64(curSource), 10)
516+
517+
t.Logf("Hold blocks from current source: %s", curSourceLabel)
518+
e.PartyStates[curSource].HoldFromBlock.Store(b.Block.Header.Number)
519+
520+
curSusCount := test.GetIntMetricValue(t, m.SuspicionRaisedTotal.WithLabelValues(curSourceLabel))
521+
curConfirmedSusCount := test.GetIntMetricValue(t, m.SuspicionConfirmedTotal.WithLabelValues(curSourceLabel))
522+
curClearedSusCount := test.GetIntMetricValue(t, m.SuspicionClearedTotal.WithLabelValues(curSourceLabel))
523+
524+
expectedDeadline := float64(time.Now().Add(time.Hour - time.Millisecond).UnixMilli())
525+
t.Log("Submit another block without receiving it")
526+
txs := workload.GenerateTransactions(t, nil, 10)
527+
block := workload.MapToOrdererBlock(0, txs)
528+
err := e.Orderer.SubmitBlock(t.Context(), block)
529+
require.NoError(t, err)
530+
b, ok := channel.NewReader(t.Context(), e.outputWithSource).ReadWithTimeout(3 * time.Second)
531+
require.False(t, ok)
532+
require.Nil(t, b)
533+
534+
suspicions := test.GetIntMetricValue(t, m.SuspicionRaisedTotal.WithLabelValues(curSourceLabel))
535+
require.Greater(t, suspicions, curSusCount)
536+
suspicionConfirmed := test.GetIntMetricValue(t, m.SuspicionConfirmedTotal.WithLabelValues(curSourceLabel))
537+
require.Equal(t, curConfirmedSusCount, suspicionConfirmed)
538+
suspicionCleared := test.GetIntMetricValue(t, m.SuspicionClearedTotal.WithLabelValues(curSourceLabel))
539+
require.Equal(t, curClearedSusCount, suspicionCleared)
540+
541+
// Verify target arrival deadline was set when suspicion was raised.
542+
targetDeadline := test.GetMetricValue(t, m.TargetArrivalDeadline.WithLabelValues(curSourceLabel))
543+
require.Greater(t, targetDeadline, expectedDeadline)
544+
545+
// Verify gap.
546+
gap := test.GetIntMetricValue(t, m.BlockGapGauge.WithLabelValues(curSourceLabel))
547+
require.Equal(t, -1, gap)
548+
549+
t.Log("Release data block")
550+
e.PartyStates[curSource].HoldFromBlock.Store(0)
551+
blk := e.WaitForBlock(t, e.output)
552+
require.NotNil(t, blk)
553+
554+
// Suspecion cleared.
555+
suspicionCleared = test.GetIntMetricValue(t, m.SuspicionClearedTotal.WithLabelValues(curSourceLabel))
556+
require.Greater(t, suspicionCleared, curClearedSusCount)
557+
targetDeadline = test.GetMetricValue(t, m.TargetArrivalDeadline.WithLabelValues(curSourceLabel))
558+
require.Zero(t, targetDeadline)
559+
560+
stopDelivery()
502561
})
503562
}
504563
}

0 commit comments

Comments
 (0)