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

Add SignalFx Event Support to SignalFx Exporter #1036

Merged
merged 1 commit into from
Sep 23, 2020
Merged

Add SignalFx Event Support to SignalFx Exporter #1036

merged 1 commit into from
Sep 23, 2020

Conversation

keitwb
Copy link
Contributor

@keitwb keitwb commented Sep 15, 2020

This consumes the logs generated by the SignalFx receiver that
come from events.

Ignore the first commit and only focus on the second. It contains dependent code from #1035 that this PR depends on to build properly.

@keitwb keitwb requested a review from a team as a code owner September 15, 2020 19:44
@project-bot project-bot bot added this to In progress in Collector Sep 15, 2020
exporter/signalfxexporter/config.go Show resolved Hide resolved
// avoid attempting to compress things that fit into a single ethernet frame
func (s *sfxClientBase) getReader(b []byte) (io.Reader, bool, error) {
var err error
if len(b) > 1500000 {
Copy link
Member

Choose a reason for hiding this comment

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

1500000 does not appear to be a single ethernet frame size. I am probablly missing something. AFAIK MTU is typically 1500 (jumbo frames can go up to 9000).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah not sure how that got changed, will fix.

exporter/signalfxexporter/dpclient.go Show resolved Hide resolved
@tigrannajaryan tigrannajaryan self-assigned this Sep 17, 2020
@tigrannajaryan
Copy link
Member

@keitwb the build is failing, please check.

@tigrannajaryan
Copy link
Member

The code lgtm, but need to fix the build errors.

@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #1036 into master will increase coverage by 0.03%.
The diff coverage is 91.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1036      +/-   ##
==========================================
+ Coverage   88.88%   88.92%   +0.03%     
==========================================
  Files         254      256       +2     
  Lines       12150    12306     +156     
==========================================
+ Hits        10800    10943     +143     
- Misses       1003     1011       +8     
- Partials      347      352       +5     
Flag Coverage Δ
#integration 75.42% <ø> (-0.11%) ⬇️
#unit 87.97% <91.01%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...nalfxexporter/translation/logdata_to_signalfxv2.go 82.75% <82.75%> (ø)
exporter/signalfxexporter/eventclient.go 89.65% <89.65%> (ø)
exporter/signalfxexporter/config.go 90.69% <100.00%> (-0.42%) ⬇️
exporter/signalfxexporter/dpclient.go 90.80% <100.00%> (+0.32%) ⬆️
exporter/signalfxexporter/exporter.go 93.50% <100.00%> (+12.55%) ⬆️
exporter/signalfxexporter/factory.go 89.47% <100.00%> (+1.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6145a10...a270039. Read the comment docs.

@tigrannajaryan
Copy link
Member

Fix the conflicts.

})

event.EventType = lr.Name()
event.Timestamp = int64(lr.Timestamp()) / 1e6
Copy link
Member

Choose a reason for hiding this comment

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

Add comment to explain time unit conversion (1e6 is not clear).

This consumes the logs generated by the SignalFx receiver that
come from events.
Collector automation moved this from In progress to Reviewer approved Sep 23, 2020
@tigrannajaryan tigrannajaryan merged commit 6782ce6 into open-telemetry:master Sep 23, 2020
Collector automation moved this from Reviewer approved to Done Sep 23, 2020
dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
The goal is to understand who is using the OpenTelemetry Collector in
production. Other CNCF projects have taken a similar approach to
highlight adoption.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants