Skip to content

Add typed events and metrics to the metadata module #88

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

Closed
13 tasks
iramiller opened this issue Feb 22, 2021 · 0 comments · Fixed by #281
Closed
13 tasks

Add typed events and metrics to the metadata module #88

iramiller opened this issue Feb 22, 2021 · 0 comments · Fixed by #281
Assignees
Labels
enhancement New feature or request metadata Metadata Module nice-to-have Features not required in upcoming milestone
Milestone

Comments

@iramiller
Copy link
Member

iramiller commented Feb 22, 2021

Summary

Implement Typed Events for Metadata Module

Problem Definition

Typed Event Conversion

The SDK recently added support for typed events and deprecated the existing style of kv event attributes. This issue is for refactoring the existing events into proto messages and wiring up typed events to replace the existing ones.

  • Replace Message Events with Typed Versions
  • Ensure all Core Keeper functions that mutate state publish an associated typed event
    • Set
    • Remove

Metric Publishing

Tracking the volume of information being handled on chain and the duration of requests is an important part of monitoring the health of the blockchain. Metrics were add. Along with ensuring proper events are published the associated metrics should also be put in place. These two efforts are linked because they typically are inserted at the same point in the code for similar reasons.

  • Being Blocker methods are appropriately timed defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker)
  • All creation, modification, and validation methods are timed defer telemetry.MeasureSince(time.Now(), "metadata", "validation|scope|specification", "get|set|validate|etc")
  • Creation of new records are counted defer telemetry.IncrCounter(1, "new", "scope|record|specification")
  • Gauge metrics are set of appropriate amount values (examples, mint/burn amounts, send amounts)
  • Appropriate labels are applied to each counter, gauge

Example Metric and Events from staking module msg_server

	if msg.Amount.Amount.IsInt64() {
		defer func() {
			telemetry.IncrCounter(1, types.ModuleName, "delegate")
			telemetry.SetGaugeWithLabels(
				[]string{"tx", "msg", msg.Type()},
				float32(msg.Amount.Amount.Int64()),
				[]metrics.Label{telemetry.NewLabel("denom", msg.Amount.Denom)},
			)
		}()
	}

	ctx.EventManager().EmitEvents(sdk.Events{
		sdk.NewEvent(
			types.EventTypeDelegate,
			sdk.NewAttribute(types.AttributeKeyValidator, msg.ValidatorAddress),
			sdk.NewAttribute(sdk.AttributeKeyAmount, msg.Amount.Amount.String()),
		),
		sdk.NewEvent(
			sdk.EventTypeMessage,
			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
			sdk.NewAttribute(sdk.AttributeKeySender, msg.DelegatorAddress),
		),
	})

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@iramiller iramiller added enhancement New feature or request metadata Metadata Module nice-to-have Features not required in upcoming milestone labels Feb 22, 2021
@iramiller iramiller added this to the 1.0.0 milestone Mar 5, 2021
@iramiller iramiller modified the milestones: 1.0.0, 1.1.0 Mar 25, 2021
@iramiller iramiller modified the milestones: 1.1.0, v1.2.0, v1.3.0 Apr 21, 2021
@dwedul-figure dwedul-figure self-assigned this Apr 29, 2021
@iramiller iramiller moved this from Todo to Done in Provenance Core Protocol Team Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request metadata Metadata Module nice-to-have Features not required in upcoming milestone
Projects
Development

Successfully merging a pull request may close this issue.

2 participants