Atlas plugin #23

sajit · 2024-08-21T21:56:20Z

What does this PR change?

Introduces method to create cost explorer query

Does this PR relate to any other PRs?

No

How will this PR impact users?

Not sure

Does this PR address any GitHub or Zendesk issues?

No

How was this PR tested?

Unit tets

Does this PR require changes to documentation?

mongodb-atlas/cmd/main.go

ameijer · 2024-08-22T12:35:04Z

Amazing, Sajit! I created an issue for tracking here: #24

ameijer

regarding project structure - I think you are actually pretty close. The main package is conventionally where the main() method is... you will always have a main method, each plugin is its own standalone go program.

I regret naming the plugin implementation package datadogplugin, i would change the mongodbatlasplugin package to just be named plugin... less typing to use that stuff, haha.

I'd also set up a test package that can hold an integration testing harness for this

mongodb-atlas/go.mod

mongodb-atlas/cmd/main.go

ameijer · 2024-08-22T12:47:36Z

mongodb-atlas/cmd/main.go

+// TODO move these types to mongodbatlasplugin package
+type CreateCostExplorerQueryPayload struct {
+	Clusters              []string `json:"clusters"`
+	EndDate               string   `json:"endDate"`


have you considered more strongly typing this with a time.Time object? GO's json serializer is very good and I bet the API would be happy with what it gets serialized to

Ive modified the function itself to take time.Time. So its more strict. Internally we are creating this struct. when I tried changing EndDate to time.Time issue I was running into was that the string was of the format yyyy-mm-dd+timestamp and mongo atlas didn't like it. Instead of figuring out how to modify the json.Marshal to marshal a time type to yyyy-mm-dd without the timestamp I just do a parse in the function itself.

mongodb-atlas/cmd/main.go

mongodb-atlas/cmd/main_test.go

ameijer · 2024-08-22T12:59:10Z

mongodb-atlas/cmd/main.go

+	fmt.Println(error)
+}
+
+func getTruth() bool {


likewise to lowercase variables in structs, this function is encapsulated in the main package

ameijer · 2024-08-22T13:06:04Z

mongodb-atlas/cmd/main.go

+
+	payloadJson, marshallingError := json.Marshal(payload)
+	if marshallingError != nil {
+		return "error", marshallingError


golang error handling is one of the most interesting and different parts of the language. Even after 5 years of it being my daily driver, I can't decide if I like it better or not than Java.

Basically, go never throws exceptions. if you do a nil access, it will panic with a segfault like C, but you can't use exception handling to deal with unexpected errors. you will see error handles like this sprinkled throughout all go code.

We have found the following approach works:

when returning an error, your "good" value should be nil if a pointer, or an empty value if not a pointer.

log the error in context (import the logging package from opencost, that will handle logger setup for you, I can point you to examples)

wrap the error before you return it. As the error gets passed up the stack, you have nested wrapped messages which function similar to the stacktrace in a java exception.

Here's how I would handle this error in this case:

if marshallingError != nil { msg := fmt.Sprintf("createCostExplorerQueryToken: error marshaling JSON payload: %v", err) log.Error(msg) return "", fmt.Errorf(msg) }

Will take point on this.

ameijer · 2024-08-22T13:07:32Z

mongodb-atlas/cmd/main.go

+	}
+	defer response.Body.Close()
+
+	fmt.Println("response Status:", response.Status)


I'd go ahead and move all this to a debug or trace level logger. This will be useful for ongoing maintenance

ameijer · 2024-08-22T13:11:36Z

mongodb-atlas/mongodbatlasplugin/mongodb-atlas-config.go

+package mongodbatlas
+
+type MongoDBAtlasConfig struct {
+	PublicKey  string


I'd go ahead and get this reading off the file system during the boot sequence. Check out how we do it in getDatadogConfig and getConfigFilePath() in that plugin. This config will grow quickly as this matures and you will be glad to get it in a file ASAP before you get trapped in env var hell world

kwombach12 · 2024-08-22T14:56:17Z

@sajit Thanks so much for your contribution! We are super excited to work with you on this. Please don't hesitate to reach out if you have any issues or questions.

sajit · 2024-08-28T21:27:29Z

@ameijer @kwombach12 Thanks for the feedback. Kindly take another look. Ive addressed most of the comments / deleted unused code. A few things to note. In main_test.go, all but 1 test are unit tests.
For, TestMain test in mongodb-atlas/cmd/main_test.go to run the execution environment , needs to have some env variables set . (its documented in the comments) . This is a true integration test - ie it will make a real call to MongoDB atlas.

sajit · 2024-08-28T21:28:25Z

Also could I get some help understanding whats happening with DCO - I believe I have used the correct commit message template. thanks in advance

sajit · 2024-08-28T21:33:19Z

mongodb-atlas/cmd/main.go

+}
+
+// pass list of orgs , start date, end date
+func createCostExplorerQueryToken(org string, startDate time.Time, endDate time.Time,


Changed startDate, endDate to be time.Time

ameijer · 2024-08-29T17:33:13Z

wow the DCO thing is weird. Have you set up GPG keys? is it related to that?

If all else fails, you can always just copy the files into a new branch

sajit · 2024-08-29T22:49:26Z

@ameijer pretty silly apparently its because DCO requires your first name and last name and my github was just my first name. lol. Anyways I think its passing DCO check now. Should be good to be reviewed.

ameijer

exciting stuff!

ameijer · 2024-10-07T17:00:40Z

mongodb-atlas/plugin/costexplorerquery.go

+	Service          string  `json:"service"`
+	UsageAmount      float32 `json:"usageAmount"`
+	UsageDate        string  `json:"usageDate"`
+	//"invoiceId":"66d7254246a21a41036ff315","organizationId":"66d7254246a21a41036ff2e9","organizationName":"Kubecost","service":"Clusters","usageAmount":51.19,"usageDate":"2024-09-01"}


can you redact the IDs here?

ameijer · 2024-10-07T17:01:03Z

mongodb-atlas/plugin/costexplorerquery.go

@@ -0,0 +1,29 @@
+package plugin
+
+type CreateCostExplorerQueryPayload struct {


ameijer · 2024-10-07T17:01:39Z

mongodb-atlas/config/atlasconfig_test.go

+	// 	if config.LogLevel != "debug" {
+	// 		t.Errorf("expected log level to be 'debug', but got: %s", config.LogLevel)
+	// 	}
+	// })


why commented?

ameijer · 2024-10-07T17:06:25Z

.gitmessage.txt

@@ -0,0 +1,2 @@
+#Commit Message template


can you remove these files?

ameijer · 2024-10-07T17:06:47Z

.vscode/launch.json

+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [


I gitignored this and removed it in sajit#4

ameijer · 2024-10-07T17:18:49Z

mongodb-atlas/cmd/main.go

+	for _, invoice := range costResponse.UsageDetails {
+		// Create a new pb.CustomCost for each Invoice
+		customCost := &pb.CustomCost{
+			Id:             invoice.InvoiceId,


this ID is actually something you assign. You can just do UUID.NewUUIDV4String() from one of the many UUID packages out there.

ameijer · 2024-10-07T17:20:06Z

mongodb-atlas/cmd/main.go

+func CreateCostExplorerQueryToken(org string, startDate time.Time, endDate time.Time,
+	client HTTPClient) (string, error) {
+	// Define the layout for the desired format
+	layout := "2006-01-02"


ameijer · 2024-10-07T17:20:21Z

mongodb-atlas/cmd/main.go

+
+	payload := atlasplugin.CreateCostExplorerQueryPayload{
+
+		EndDate:       endDateString,


rm space above this

ameijer · 2024-10-07T17:22:12Z

mongodb-atlas/cmd/main.go

+	// Iterate over the UsageDetails in CostResponse
+	for _, invoice := range costResponse.UsageDetails {
+		// Create a new pb.CustomCost for each Invoice
+		customCost := &pb.CustomCost{


can you set UsageUnit as USD on this too?

on the metadata field, can you set an interpolated: true key/value?

another additional field that is super important is the ProviderID. We can sort of synthesize one. Maybe the providerID can be <orgid>/<invoiceid>/<service>

ameijer · 2024-10-07T17:23:29Z

mongodb-atlas/cmd/main.go

+		customCost := &pb.CustomCost{
+			Id:             invoice.InvoiceId,
+			AccountName:    invoice.OrganizationName,
+			ChargeCategory: invoice.Service,


I would actually set ResourceName to the invoice.Service

Init commit cmd files intermediate stop Changing signature adding scripts to gitignore address review comments address review comments ignore mock test failing test Signed-off-by: sajit <[email protected]> fixed test Make test optional Signed-off-by: sajit <[email protected]> adding tests Signed-off-by: sajit <[email protected]> Added tests Signed-off-by: sajit <[email protected]> Delete unused struct Signed-off-by: sajit <[email protected]> Signed-off-by: sajit <[email protected]>

Merge Alex's PR to continue work on branch Signed-off-by: sajit <[email protected]>

Signed-off-by: sajit <[email protected]>

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

prep for testing, fix build Signed-off-by: Alex Meijer <[email protected]>

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

Signed-off-by: sajit <[email protected]>

* OpenAI Plugin MVP Signed-off-by: Alex Meijer <[email protected]> * code review fixes Signed-off-by: Alex Meijer <[email protected]> * add MAINTAINERS Signed-off-by: Alex Meijer <[email protected]> * only attempt to read if no error Signed-off-by: Alex Meijer <[email protected]> --------- Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

Signed-off-by: sajit <[email protected]>

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

* implement validator, small bugfix Signed-off-by: Alex Meijer <[email protected]> * additional testing Signed-off-by: Alex Meijer <[email protected]> --------- Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

Signed-off-by: sajit <[email protected]>

…nnumkal <[email protected]> Signed-off-by: sajit <[email protected]>

ameijer · 2024-10-16T20:16:39Z

obsoleted by #41

sajit commented Aug 21, 2024

View reviewed changes

mongodb-atlas/cmd/main.go Outdated Show resolved Hide resolved

github-actions bot added the needs-follow-up label Aug 22, 2024

ameijer linked an issue Aug 22, 2024 that may be closed by this pull request

OpenCost Plugin For MongoDB Atlas #24

Closed

ameijer reviewed Aug 22, 2024

View reviewed changes

sajit marked this pull request as draft August 27, 2024 20:55

sajit force-pushed the atlas-plugin branch 3 times, most recently from 3683257 to 1225ea2 Compare August 27, 2024 21:30

sajit marked this pull request as ready for review August 28, 2024 21:19

sajit force-pushed the atlas-plugin branch from b6f2224 to f87c609 Compare August 28, 2024 21:32

sajit commented Aug 28, 2024

View reviewed changes

sajit force-pushed the atlas-plugin branch from f87c609 to 9a4b2b4 Compare August 29, 2024 22:46

ameijer reviewed Oct 7, 2024

View reviewed changes

sajit force-pushed the atlas-plugin branch from 923b29d to df1a5c6 Compare October 12, 2024 01:41

sajit and others added 10 commits October 16, 2024 10:40

boilerplate for plugin interface (#1)

09bb7a3

Merge Alex's PR to continue work on branch Signed-off-by: sajit <[email protected]>

Added tests for atlasconfig.go

5fe974f

Signed-off-by: sajit <[email protected]>

Get costs

7e00bca

Signed-off-by: sajit <[email protected]>

Signed-off-by: Sajit Kunnumkal <[email protected]>

5d26705

Signed-off-by: sajit <[email protected]>

Add unit tests

d8f1adb

Signed-off-by: sajit <[email protected]>

more tests

cfd8a56

Signed-off-by: sajit <[email protected]>

migrate mongo plugin to use harness (#4)

bdf1210

Signed-off-by: sajit <[email protected]>

fix atlas_config_test

24aed3e

Signed-off-by: sajit <[email protected]>

have a working version. Need to change the function signature

0f305cf

Signed-off-by: sajit <[email protected]>

sajit and others added 18 commits October 16, 2024 10:40

working unit tests

9013769

Signed-off-by: sajit <[email protected]>

unit tests

011320b

Signed-off-by: sajit <[email protected]>

optional integration test

493bc4e

Signed-off-by: sajit <[email protected]>

Fixing DCO error Signed-off-by: sajit <[email protected]>

2581ed9

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

checkout current branch for now

7e562b1

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

JPHM Signed-off-by: sajit <[email protected]>

dbaf3f6

prep for testing, fix build Signed-off-by: Alex Meijer <[email protected]>

repro on branch Signed-off-by: Alex Meijer <[email protected]>

2cde737

use branch

f74a1f0

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

us PAT to commit Signed-off-by: Alex Meijer <[email protected]>

9f5de8c

update manifest

7d679a7

Signed-off-by: sajit <[email protected]>

update manifest

1b9f1f7

Signed-off-by: sajit <[email protected]>

update manifest

c8a4a44

Signed-off-by: sajit <[email protected]>

fix harness to support multiple plugins (opencost#39)

9ff32f3

Signed-off-by: Alex Meijer <[email protected]> Signed-off-by: sajit <[email protected]>

fixing tests; remove .gitmessage.txt from source control

27c8385

Signed-off-by: sajit <[email protected]>

fix tests

b81730a

Signed-off-by: sajit <[email protected]>

address PR comments: change error to warnings Signed-off-by: Sajit Ku…

52e0d61

…nnumkal <[email protected]> Signed-off-by: sajit <[email protected]>

sajit force-pushed the atlas-plugin branch from e64b73e to 52e0d61 Compare October 16, 2024 15:17

ameijer closed this Oct 16, 2024

sajit deleted the atlas-plugin branch October 19, 2024 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Atlas plugin #23

Atlas plugin #23

sajit commented Aug 21, 2024

ameijer commented Aug 22, 2024

ameijer left a comment

ameijer Aug 22, 2024

sajit Aug 28, 2024

ameijer Aug 22, 2024

ameijer Aug 22, 2024

sajit Aug 26, 2024

ameijer Aug 22, 2024

ameijer Aug 22, 2024

kwombach12 commented Aug 22, 2024

sajit commented Aug 28, 2024

sajit commented Aug 28, 2024

sajit Aug 28, 2024

ameijer commented Aug 29, 2024

sajit commented Aug 29, 2024

ameijer left a comment

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer Oct 7, 2024

ameijer commented Oct 16, 2024

		@@ -0,0 +1,29 @@
		package plugin

		type CreateCostExplorerQueryPayload struct {


		payload := atlasplugin.CreateCostExplorerQueryPayload{

		EndDate: endDateString,

Atlas plugin #23

Atlas plugin #23

Conversation

sajit commented Aug 21, 2024

What does this PR change?

Does this PR relate to any other PRs?

How will this PR impact users?

Does this PR address any GitHub or Zendesk issues?

How was this PR tested?

Does this PR require changes to documentation?

ameijer commented Aug 22, 2024

ameijer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwombach12 commented Aug 22, 2024

sajit commented Aug 28, 2024

sajit commented Aug 28, 2024

Choose a reason for hiding this comment

ameijer commented Aug 29, 2024

sajit commented Aug 29, 2024

ameijer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ameijer commented Oct 16, 2024