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.

Sign up for GitHub

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

Jump to bottom

feat: traces block height and timestamp for InfluxDB #1062

Merged
merged 9 commits into from
Aug 17, 2023
Merged

feat: traces block height and timestamp for InfluxDB #1062

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9109b54
adds a TODO
staheri14 Aug 16, 2023
f1eced2
introduces block table schema
staheri14 Aug 16, 2023
64687de
writes block timestamp
staheri14 Aug 16, 2023
061b926
makes a comment
staheri14 Aug 16, 2023
e4dc897
adds timestamp field to the schema description
staheri14 Aug 16, 2023
7bdbf88
fixes linter issue
staheri14 Aug 16, 2023
04ce8da
deletes excess spaces
staheri14 Aug 16, 2023
1000428
fixes goimports problem
staheri14 Aug 17, 2023
1d29ac2
adds _time suffix to the table name
staheri14 Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions consensus/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,9 @@ func (cs *State) finalizeCommit(height int64) {
// must be called before we update state
cs.recordMetrics(height, block)

// trace some metadata about the block
schema.WriteBlock(cs.traceClient, block.Header.Height, block.Header.Time)

// NewHeightStep!
cs.updateToState(stateCopy)

Expand Down
22 changes: 22 additions & 0 deletions pkg/trace/schema/consensus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package schema

import (
"time"

cstypes "github.com/tendermint/tendermint/consensus/types"
"github.com/tendermint/tendermint/p2p"
"github.com/tendermint/tendermint/pkg/trace"
Expand All @@ -12,6 +14,7 @@ func ConsensusTables() []string {
return []string{
RoundStateTable,
BlockPartsTable,
BlockTable,
}
}

Expand Down Expand Up @@ -81,3 +84,22 @@ func WriteBlockPart(
TransferTypeFieldKey: transferType,
})
}

const (
// BlockTable is the name of the table that stores metadata about consensus blocks.
// following schema:
//
// | time | height | timestamp |
BlockTable = "consensus_block"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps?

Suggested change
BlockTable = "consensus_block"
BlockTable = "consensus_block_time"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 1d29ac2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I initially avoided the _time suffix was that we may actually want to trace further attributes for each block. Nevertheless, I incorporated your suggestion!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may actually want to trace further attributes for each block.

exactly, this was actually the same reason I think we should change it. If we want further metrics, then we will use a new table, but the name "consensus_block" is already used by the block time

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may actually want to trace further attributes for each block.

exactly, this was actually the same reason I think we should change it. If we want further metrics, then we will use a new table, but the name "consensus_block" is already used by the block time

So, does this mean that for each additional metric associated with the block, a new table will need to be created? For instance, if we intend to store the DataHash of the block header, would it be necessary to establish another table with a |time|height|data_hash| schema? Not that it is a big issue, but there seems to be potential redundancy, such as the block height being recorded in multiple tables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh I see okay that makes sense

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to you on if we want to change it back. We can also change it when/if we add more things to this table.


// TimestampFieldKey is the name of the field that stores the time at which
// the block is proposed.
TimestampFieldKey = "timestamp"
)

func WriteBlock(client *trace.Client, height int64, blockTimestamp time.Time) {
client.WritePoint(BlockTable, map[string]interface{}{
HeightFieldKey: height,
TimestampFieldKey: blockTimestamp,
})
}
Loading