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

Handle sumo headers #6

Merged
merged 1 commit into from
Dec 3, 2020
Merged

Handle sumo headers #6

merged 1 commit into from
Dec 3, 2020

Conversation

sumo-drosiek
Copy link
Owner

Handle sumo headers

@sumo-drosiek sumo-drosiek force-pushed the drosiek-sumo-headers-pr branch from af256b0 to 69da48f Compare November 27, 2020 17:28
pmalek-sumo
pmalek-sumo reviewed Nov 30, 2020
View reviewed changes
exporter/sumologicexporter/source_format.go
template string
}

func newSourceFormat(r *regexp.Regexp, text string) sourceFormat {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comment what arguments does this func expect, i.e. what regex and what string and what are they used for.

exporter/sumologicexporter/filter.go Outdated Show resolved Hide resolved
exporter/sumologicexporter/filter.go Outdated Show resolved Hide resolved
exporter/sumologicexporter/fields.go Outdated Show resolved Hide resolved
exporter/sumologicexporter/fields.go Outdated Show resolved Hide resolved
@sumo-drosiek sumo-drosiek force-pushed the drosiek-sumo-headers-pr branch 2 times, most recently from 7dff1de to d915b86 Compare December 1, 2020 16:26
@sumo-drosiek sumo-drosiek force-pushed the drosiek-sumo-headers-pr branch from d915b86 to d5369d8 Compare December 2, 2020 10:48
@sumo-drosiek sumo-drosiek force-pushed the drosiek-sumo-headers-pr branch from d5369d8 to 2e3f50f Compare December 2, 2020 12:37
pmalek-sumo
pmalek-sumo reviewed Dec 3, 2020
View reviewed changes
Copy link
Collaborator

@pmalek-sumo pmalek-sumo left a comment

Choose a reason for hiding this comment

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

Ok, other than those small suggestions I think this is quite good.
I still haven't figured out a way to change functions' signatures so that we don't have to pass fields{} whenever we want to send stuff without logs :(

exporter/sumologicexporter/sender.go
config *Config
client *http.Client
filter filter
ctx context.Context
Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing for sure: context.Context is not meant to be stored and reused.
Change send's signature (I'm OK with doing this in a separate PR) to accept ctx as first parameter and for starters you can pass there context.Background() or more explicitly context.TODO()

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is intended to not pass context through 5 functions, Sender works totally synchronized so context will be used same way as passing it by function signature

Copy link
Owner Author

Choose a reason for hiding this comment

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

Will address it in another PR, probably with enabling logs

exporter/sumologicexporter/source_format_test.go
)

func getTestSourceFormat(t *testing.T, template string) sourceFormat {
r, err := regexp.Compile(`\%\{(\w+)\}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
r, err := regexp.Compile(`\%\{(\w+)\}`)
r, err := regexp.Compile(sourceRegex)

Copy link
Owner Author

@sumo-drosiek sumo-drosiek Dec 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It won't catch accidental change of the const

exporter/sumologicexporter/source_format_test.go
template: "%s/test",
}

r, err := regexp.Compile(`\%\{(\w+)\}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
r, err := regexp.Compile(`\%\{(\w+)\}`)
r, err := regexp.Compile(sourceRegex)

@sumo-drosiek sumo-drosiek merged commit 6293eb8 into drosiek-sumo-headers Dec 3, 2020
@sumo-drosiek sumo-drosiek mentioned this pull request Dec 4, 2020
Merged
sumo-drosiek pushed a commit that referenced this pull request Jan 8, 2021
@kohrapha
[awsemfexporter] Implement metric filtering and dimension setting (#1503
6b97426 
)

* Create MetricDeclaration struct

* Implement dimension dedup logic when adding rolledup dimensions (#6)

* Implement dimension dedup logic when adding rolledup dimensions

* Remove duplicated dimensions in metric declaration

* Create benchmark for filtering with metric declarations and use assertCwMeasurementEqual for tests

* Move helper test code to the top of file

* Update dimRollup test

* Aggregate dimensions and perform dedup

* Minor code change

* Fix test failure from merging new changes
sumo-drosiek pushed a commit that referenced this pull request Apr 20, 2021
@rockb1017
Add ResourceAttributes to Splunk Event (open-telemetry#2843)
c4a207e 
**Description:** 
Fixes open-telemetry#2799

**Testing:** 
```
ResourceLog #6
Resource labels:
     -> k8s.container.name: STRING(loggen)
     -> k8s.pod.uid: STRING(f7880852-38f9-4158-b37a-41b3436a3b95)
     -> k8s.namespace.name: STRING(default)
     -> k8s.pod.startTime: STRING(2021-03-24 23:18:27 +0000 UTC)
     -> k8s.node.name: STRING(ip-192-168-68-130.us-west-1.compute.internal)
     -> k8s.pod.labels.hello: STRING(world)
     -> k8s.pod.labels.app: STRING(appName)
     -> k8s.pod.annotations.splunk.com/index: STRING(main)
     -> host.name: STRING(ip-192-168-68-130.us-west-1.compute.internal)
     -> com.splunk.sourcetype: STRING(loggen)
     -> com.splunk.index: STRING(main)
InstrumentationLibraryLogs #0
InstrumentationLibrary
LogRecord #0
Timestamp: 1616627909498109658
Severity: Undefined
ShortName:
Body: {
     -> log: STRING(EPS: 99
)
}
Attributes:
     -> k8s.namespace.name: STRING(default)
     -> k8s.pod.name: STRING(loggen-65dvk)
     -> run_id: STRING(0)
     -> k8s.container.name: STRING(loggen)
```

![image](https://user-images.githubusercontent.com/12387289/112396117-d7f79780-8cbc-11eb-879f-07bb1eb4413a.png)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmalek-sumo pmalek-sumo pmalek-sumo left review comments

@pmm-sumo pmm-sumo Awaiting requested review from pmm-sumo pmm-sumo is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sumo-drosiek @pmalek-sumo