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.

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

refactor: use v2/metrics for cli honeycomb events #1522

Merged
merged 7 commits into from
Jan 24, 2024
Merged

Conversation

dmurray-lacework
Copy link
Collaborator

@dmurray-lacework dmurray-lacework commented Jan 24, 2024

Summary

Refactor honeycomb prod events sent by CLI to use v2/metrics instead of direct to honeycomb.

  • Note -dev events have already been using the v2/metrics since v1.44.0 see lacework-cli-dev dataset.

How did you test this change?

  • Run CLI commands observe honeycomb dataset
  • Ensure Trace with multiple spans works as expected

Issue

https://lacework.atlassian.net/browse/GROW-2705

@dmurray-lacework dmurray-lacework requested a review from a team as a code owner January 24, 2024 10:47
@dmurray-lacework dmurray-lacework force-pushed the GROW-2705 branch 2 times, most recently from 8024f19 to 93614ca Compare January 24, 2024 12:55
Signed-off-by: Darren Murray <darren.murray@lacework.net>
Signed-off-by: Darren Murray <darren.murray@lacework.net>
Signed-off-by: Darren Murray <darren.murray@lacework.net>
cli/cmd/cache.go Outdated
@@ -132,7 +132,7 @@ func (c *cliState) EraseCachedToken() error {
}

func (c *cliState) ReadCachedToken() {
if c.noCache {
if c.noCache || c.Cache == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel you catch some issues plugin this in, I am now against these additions but I want to understand why we need to check for the cache here and also, initialize the api somewhere else that is not the root.go?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prior to this we could send Honeycomb events without the client being initialized.

In the persistent prerun we always sent a Honeycomb event, say for example lacework help.
We then got an event in honeycomb with the cmd,version but no account/subaccount or any client data.

Now we cannot do that. We need Lacework auth to send a honeycomb event. This also means those honeycomb events with no client data will not be sent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The check for cache being nil is for some of the help tests that were failing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've removed initing the client in sendhoneycomb event. If there is no client we just return.

Signed-off-by: Darren Murray <darren.murray@lacework.net>
Signed-off-by: Darren Murray <darren.murray@lacework.net>
Signed-off-by: Darren Murray <darren.murray@lacework.net>
Signed-off-by: Darren Murray <darren.murray@lacework.net>
@dmurray-lacework dmurray-lacework merged commit 656a1d3 into main Jan 24, 2024
4 checks passed
@dmurray-lacework dmurray-lacework deleted the GROW-2705 branch January 24, 2024 19:37
@lacework-releng lacework-releng mentioned this pull request Jan 25, 2024
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