Skip to content

Replication observability metrics#142

Merged
nitesh3108 merged 11 commits into
mainfrom
replication-observability-metrics
Oct 8, 2024
Merged

Replication observability metrics#142
nitesh3108 merged 11 commits into
mainfrom
replication-observability-metrics

Conversation

@nitesh3108

@nitesh3108 nitesh3108 commented Oct 8, 2024

Copy link
Copy Markdown
Contributor

PR Submission checklist

It enables API support for volume_mirror_transfer_rate_cma_view which will be helpful to pull the Replication related stats for Sync and Metro at volume level.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1443

Common PR Checklist:

  • Have you made sure that the code compiles?
  • Have you commented your code, particularly in hard-to-understand areas
  • Did you run tests in a real Kubernetes cluster?
  • Have you maintained backward compatibility

Description of your changes:

  • Executed gopowerstore unit tests

Earlier unit-test report

PASS
coverage: 82.2% of statements
ok      github.com/dell/gopowerstore    0.069s  coverage: 82.2% of statements

Current unit-test report

=== RUN   TestClientIMPL_VolumeMirrorTransferRate
--- PASS: TestClientIMPL_VolumeMirrorTransferRate (0.00s)

PASS
coverage: 83.8% of statements
ok      github.com/dell/gopowerstore    0.064s  coverage: 83.8% of statements

Comment thread metrics.go
Comment thread metrics.go Outdated
// VolumeMirrorTransferRate - Volume Mirror Transfer Rate
func (c *ClientIMPL) VolumeMirrorTransferRate(ctx context.Context, entityID string) ([]VolumeMirrorTransferRateResponse, error) {
var resp []VolumeMirrorTransferRateResponse
err := c.mirrorTransferRate(ctx, &resp, entityID, 2000)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this 2000 be a constant value instead of hardcoding it here like this way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@nitesh3108 nitesh3108 force-pushed the replication-observability-metrics branch from 863ab1e to 04451e6 Compare October 8, 2024 12:38
adarsh-dell
adarsh-dell previously approved these changes Oct 8, 2024

@adarsh-dell adarsh-dell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

Comment thread metrics.go
if err != nil {
err = WrapErr(err)
}
customHeader.Del("DELL-VISIBILITY")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
customHeader.Del("DELL-VISIBILITY")
customHeader.Del("DELL-VISIBILITY")
apiClient.SetCustomHTTPHeaders(customHeader)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

adarsh-dell
adarsh-dell previously approved these changes Oct 8, 2024
@nitesh3108 nitesh3108 merged commit 6990d1b into main Oct 8, 2024
@nitesh3108 nitesh3108 deleted the replication-observability-metrics branch October 8, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants