Amend issue when getting agent host and port #230
Conversation
estringana
force-pushed
the
estringana/amend-agent-host-port-issue
branch
from
0e1611c
to
d640a14
Compare
src/extension/commands/client_init.c
Outdated
| dd_mpack_write_lstr(w, "port"); | ||
| mpack_write_uint(w, get_DD_TRACE_AGENT_PORT()); | ||
| mpack_write_uint(w, port); |
There was a problem hiding this comment.
You could probably split all of this new code into its own separate function, just for simplicity.
src/extension/commands/client_init.c
Outdated
| if (agent_host && ZSTR_LEN(agent_host) > 0) { | ||
| host = ZSTR_VAL(agent_host); | ||
| } else if (agent_url && ZSTR_LEN(agent_url) > 0) { | ||
| php_url *parsed_url = php_url_parse(ZSTR_VAL(agent_url)); |
There was a problem hiding this comment.
php_url needs to be freed after with php_url_free.
estringana
force-pushed
the
estringana/amend-agent-host-port-issue
branch
from
9132263
to
7341678
Compare
estringana
force-pushed
the
estringana/amend-agent-host-port-issue
branch
from
7341678
to
eef3de2
Compare
Codecov Report
@@ Coverage Diff @@
## master #230 +/- ##
==========================================
+ Coverage 64.10% 71.55% +7.44%
==========================================
Files 88 25 -63
Lines 5606 3290 -2316
Branches 1776 738 -1038
==========================================
- Hits 3594 2354 -1240
+ Misses 948 552 -396
+ Partials 1064 384 -680
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 63 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/extension/commands/client_init.c
Outdated
| @@ -19,6 +20,9 @@ | |||
| #include "mpack-node.h" | |||
| #include "mpack-writer.h" | |||
|
|
|||
| static const unsigned int DEFAULT_AGENT_PORT = 8126; | |||
| static const unsigned int MAX_TCP_PORT_ALLOWED = 65535; | |||
There was a problem hiding this comment.
Use UINT16_MAX instead since this is a well known limit.
src/extension/commands/client_init.c
Outdated
| } | ||
| } | ||
|
|
||
| void extract_url(url *out) |
There was a problem hiding this comment.
I think you misunderstood, I meant that you should just parse and add to msgpack here not to recreate everything with custom types. For example, something like this:
| @@ -19,6 +20,9 @@ | |||
| #include "mpack-node.h" | |||
| #include "mpack-writer.h" | |||
|
|
|||
| static const unsigned int DEFAULT_AGENT_PORT = 8126; | |||
There was a problem hiding this comment.
What about DEFAULT_AGENT_HOST?
estringana
force-pushed
the
estringana/amend-agent-host-port-issue
branch
from
d40cf95
to
9de5ddc
Compare
Description
Add defaults to agent host and port. It also parsed
DD_TRACE_AGENT_URLMotivation
Additional Notes
Describe how to test your changes
Readiness checklist
Release checklist