[GitHub] [apisix] spacewander commented on pull request #7007: change(opentelemetry): the default network port for OTLP/HTTP is changed to 4318

[GitBox <gi...@apache.org>]

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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org