spacewander commented on PR #7007: URL: https://github.com/apache/apisix/pull/7007#issuecomment-1120656844
> > This port change problem should be covered with test cases > > I think it is not necessary to add test coverage related to the default port, and the success or failure of the report will not affect the operation of APISIX. You can see that the `do_request` function does not return any relevant error information, it just output a log. Look: https://github.com/yangxikun/opentelemetry-lua/blob/main/lib/opentelemetry/trace/exporter/http_client.lua#L30 cc @spacewander In the test, the `do_request` is replaced to a stub: https://github.com/apache/apisix/blob/0d1280cde7fcd463f2e86941935e053561bcf331/t/plugin/opentelemetry2.t#L43. So changing the port won't affect the behavior of the test, and the correctness of trace reporting is covered by lua-opentelemetry itself. On the other hand, as the default network port for OTLP/HTTP is changed from 4317 to 4318 in https://github.com/open-telemetry/opentelemetry-specification/pull/1970, it is necessary to change APISIX to adapt to the new break change. CC the author of lua-opentelemetry and this plugin: @yangxikun -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
