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

writer: new option to log rejected spans #1922

Merged
merged 4 commits into from Sep 22, 2021

Conversation

jtmal-signalfx
Copy link
Contributor

No description provided.

}).Error("Error shipping spans to SignalFx")
var meta log.Fields
if sw.conf.LogTraceSpansFailedToShip {
jsonEncodedSpans, _ := json.Marshal(spans)
Copy link
Contributor

Choose a reason for hiding this comment

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

err should be checked or this can panic. Also if we failed to ship them it is possible json.Marshal could fail as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think jsonEncodedSpans would be nil if there were an err so payload below would just show nil. A json marshal error is pretty hard to imagine though since we aren't using generic data structures in the spans, just a well-defined span format that we know serializes fine.

jsonEncodedSpans, _ := json.Marshal(spans)
meta = log.Fields{
"error": err,
"payload": jsonEncodedSpans,
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this case will be enabled for debugging purposes only so it is OK to log entire batches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this option is only to be enabled when investigating tooManySpansInTrace as the backend gets rid of these

@@ -154,6 +154,7 @@ The **nested** `writer` config object has the following fields:
| `logDatapoints` | no | bool | If the log level is set to `debug` and this is true, all datapoints generated by the agent will be logged. (**default:** `false`) |
| `logEvents` | no | bool | The analogue of `logDatapoints` for events. (**default:** `false`) |
| `logTraceSpans` | no | bool | The analogue of `logDatapoints` for trace spans. (**default:** `false`) |
| `LogTraceSpansFailedToShip` | no | bool | If `true`, traces and spans which weren't successfully received by the backend, will be logged as json. (**default:** `false`) |
Copy link
Contributor

Choose a reason for hiding this comment

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

You have to run make docs in a linux env to properly generate this, not manually edited.

@keitwb
Copy link
Contributor

keitwb commented Sep 22, 2021

@jtmal-signalfx see if you can figure out what is messing with the spaces in the docs that is causing the docs_test to fail. It might be that your editor is doing formatting on it unintentionally.

@keitwb keitwb merged commit 3ac96e2 into signalfx:main Sep 22, 2021
@jtmal-signalfx jtmal-signalfx deleted the feat/log-rejected-spans branch September 23, 2021 09:31
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.

None yet

3 participants