Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Add ipv6 support #339

Merged
merged 3 commits into from
Sep 14, 2021
Merged

Add ipv6 support #339

merged 3 commits into from
Sep 14, 2021

Conversation

alvassin
Copy link
Contributor

@alvassin alvassin commented Sep 12, 2021

Added ability to report spans to ipv6 only hosts using:

  • config local_agent.reporting_host_use_ipv6 option
  • JAEGER_AGENT_HOST_USE_IPV6 environment variable.
  • if none of above options are not specified, jaeger client checks if host behind reporting_host is ipv6 only.

Resolves #337

@alvassin alvassin changed the title Add ipv6 support, resolves #337 Add ipv6 support Sep 12, 2021
@@ -303,6 +303,25 @@ def local_agent_reporting_host(self) -> str:
else:
return DEFAULT_REPORTING_HOST

@property
def local_agent_reporting_host_use_ipv6(self) -> bool:
Copy link
Member

@yurishkuro yurishkuro Sep 12, 2021

Choose a reason for hiding this comment

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

Since you're automatically determining if the address is IPv6-only, what is the value proposition of this new property? To put it differently, what is the downside of not exposing it and always handling the v4/v6 automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resonable. Added _create_socket method to TUDPTransport class, that can be overriden, removed config options.

Also, socket.set_blocking() requires boolean, so i removed the following block (perhaps there is a reason to keep it):

if blocking:
    blocking = 1
else:
    blocking = 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests

@codecov
Copy link

codecov bot commented Sep 12, 2021

Codecov Report

Merging #339 (bb59e4e) into master (1341016) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage   95.30%   95.32%   +0.01%     
==========================================
  Files          25       25              
  Lines        2046     2053       +7     
  Branches      275      275              
==========================================
+ Hits         1950     1957       +7     
  Misses         61       61              
  Partials       35       35              
Impacted Files Coverage Δ
jaeger_client/TUDPTransport.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1341016...bb59e4e. Read the comment docs.

alvassin and others added 3 commits September 13, 2021 22:15
Signed-off-by: Alexander Vasin <hi@alvass.in>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro merged commit aed18e8 into jaegertracing:master Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IPv6 support
2 participants