improve exporter module documentation and readme #292
Conversation
Codecov Report
@@ Coverage Diff @@
## main #292 +/- ##
==========================================
- Coverage 37.24% 32.33% -4.91%
==========================================
Files 46 42 -4
Lines 3257 3470 +213
==========================================
- Hits 1213 1122 -91
- Misses 2044 2348 +304
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -69,15 +71,34 @@ | |||
| -define(DEFAULT_PORT, 4317). | |||
| -define(DEFAULT_TRACES_PATH, "v1/traces"). | |||
|
|
|||
| -record(state, {protocol :: grpc | http_protobuf | http_json, | |||
| -type headers() :: [{unicode:chardata(), unicode:chardata()}]. | |||
| -type scheme() :: http | https. | |||
There was a problem hiding this comment.
Do these get parsed to atoms? Line 70 has scheme as a string.
There was a problem hiding this comment.
I was thinking it was all parsed into atoms but ultimately it needs to be a string for the call to uri_string:normalize. Except grpcbox parses it to an atom.. ugh, guess its both.
|
|
||
| ``` erlang | ||
| {opentelemetry_exporter, | ||
| [{otlp_endpoint, "https://api.honeycomb.io:443"}, |
There was a problem hiding this comment.
it might be good to add a note that SSL options aren't provided with this config. There's a new way to configure them and we could point out to https://erlef.github.io/security-wg/secure_coding_and_deployment_hardening/ssl for ways to safely configure it.
There was a problem hiding this comment.
Looks like I need to update the actual module to accept SSL args first though? And then it'd be based on these docs? https://erlef.github.io/security-wg/secure_coding_and_deployment_hardening/inets
There was a problem hiding this comment.
I thought #266 accomplished that already
There was a problem hiding this comment.
Only for grpc. Looks like httpc never got any update.
There was a problem hiding this comment.
ah damn. Then yeah it'd need similar support. I guess we can't hold back the PR for this though, and it'd be better tracked as a follow-up. Approving with the assumption the typespec in the other comment will be handled.
No description provided.