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: add new command to send test data to Observe #113

Merged
merged 5 commits into from
Nov 4, 2024
Merged

feat: add new command to send test data to Observe #113

merged 5 commits into from
Nov 4, 2024

Conversation

obs-gh-mattcotter
Copy link
Collaborator

@obs-gh-mattcotter obs-gh-mattcotter commented Oct 23, 2024 •
edited
Loading

Description

OB-32242 Create a new sub command to send test JSON data directly to Observe via HTTP post API.

Example usage:

$ ./observe-agent send-test-data --data '{"testKey": "test data"}'
Using config file: /etc/observe-agent/observe-agent.yaml
Successfully sent test data. Saw response: {"ok":true}

Example result:
Screenshot 2024-10-23 at 2 10 24 PM

Checklist

  • Created tests which fail without the change (if possible)
  • Extended the README / documentation, if necessary

obs-gh-mattcotter
obs-gh-mattcotter commented Oct 23, 2024
View reviewed changes
internal/commands/sendtestdata/sendtestdata.go Outdated
Comment on lines 14 to 19
const TestDataPath = "/observe-agent/test"

var defaultTestData = map[string]any{
"hello": "world",
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are these defaults okay?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes totally ok

internal/commands/sendtestdata/postdata.go Show resolved Hide resolved
obs-gh-alexlew
obs-gh-alexlew reviewed Oct 23, 2024
View reviewed changes
internal/commands/sendtestdata/sendtestdata.go Outdated
"github.com/spf13/cobra"
)

const TestDataPath = "/observe-agent/test"
Copy link
Collaborator

Choose a reason for hiding this comment

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

curl https://175914298205.collect.observe-eng.com/v1/http \
 -H "Authorization: Bearer $TOKEN" \
 -H "Content-type: application/json" \
 -d '{"data": "hello world"}'

request should look like this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is how the POST request is structured, but I was getting a path item of "/" in extra even when not specifying a path, so I added this for clarity. You can see my example screenshot in the PR description. Lmk if there's a way to exclude the extra all together! I couldn't find it in our docs though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hopefully the test I added can clarify this! If not, I might be misunderstanding your request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why you are sending to https://observe-agent/test/ , I think maybe Alex meant that you can send the testdata to a hardcoded collect path of that specific test customer?

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 renamed this to extra path; the post is being made to eg. https://175914298205.collect.observe-eng.com/v1/http/observe-agent/test. In our http API, it seems that you always get a field in the extra data with the path that comes after /v1/http, so I wanted to have something that stuck out and made it clear instead of just /. Does that clarify this?

obs-gh-enricogiorio
obs-gh-enricogiorio reviewed Oct 28, 2024
View reviewed changes
Copy link
Collaborator

@obs-gh-enricogiorio obs-gh-enricogiorio left a comment

Choose a reason for hiding this comment

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

LGTM, just double check the hardcoded URL

internal/commands/sendtestdata/postdata.go Outdated
return bodyString, nil
}

func PostTestDataToObserve(data any, path string, v *viper.Viper) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Honestly I'd just call it PostDataToObserve 😆

internal/commands/sendtestdata/postdata_test.go
import (
"testing"

"github.com/jarcoal/httpmock"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah httpmock is the best way to do this!

internal/commands/sendtestdata/sendtestdata.go Outdated
"github.com/spf13/cobra"
)

const TestDataPath = "/observe-agent/test"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand why you are sending to https://observe-agent/test/ , I think maybe Alex meant that you can send the testdata to a hardcoded collect path of that specific test customer?

internal/commands/sendtestdata/sendtestdata.go Outdated
Comment on lines 14 to 19
const TestDataPath = "/observe-agent/test"

var defaultTestData = map[string]any{
"hello": "world",
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes totally ok

@obs-gh-mattcotter obs-gh-mattcotter merged commit 1ff7c5c into main Nov 4, 2024
8 checks passed
@obs-gh-mattcotter obs-gh-mattcotter deleted the OB-32242-send-test-data branch November 4, 2024 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obs-gh-enricogiorio obs-gh-enricogiorio obs-gh-enricogiorio approved these changes

@obs-gh-alexlew obs-gh-alexlew Awaiting requested review from obs-gh-alexlew

Assignees

@obs-gh-enricogiorio obs-gh-enricogiorio

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@obs-gh-mattcotter @obs-gh-alexlew @obs-gh-enricogiorio