Support specifying AWS_XRAY_DAEMON_ADDRESS as hostname:port as well as ip:port

[bittercoder]

Feature request:

Having to specify AWS_XRAY_DAEMON_ADDRESS as ip:port is often no ideal if the IP address is somewhat dynamic - it would be great if hostname:port as also supported.

[yogiraj07]

Hi @bittercoder ,

Thank you for the feedback. Can you please let us know your use case. I would take this as a feature request.

Best,
Yogi

[bittercoder]

One of the simple ones is if you have x-ray daemon as a side car linked container, I would rather specify the address as "xray-daemon:2000"

But more generally I just don't want to have to specify the IP address or deal with having to role containers if the address of the x-ray daemon changes for some reason.

So I guess I'm interested in not just having hostname:port work, but ensuring that once resolved the address lookup is only cached for say a minute before being resolved again.

These are more reasons for being able to replace the Segment emitter with your own implementation as well - so these implementation details could be something I can manage myself.

[yogiraj07]

Hi @bittercoder ,

Thanks for letting us know the use case. We take this as a feature request. Please feel free to submit a pull request, we would be happy to review it.

Best,
Yogi

[emmekappa]

I have the same issue here

[emmekappa]

I'm working on a PR here https://github.com/emmekappa/aws-xray-sdk-dotnet/tree/hostname_support

[bittercoder]

Great to hear your going to work on a improvement for this @emmekappa 👍

I've done something similar to this in the past which may or may not be useful to you:

public class HostEndpoint
{
	private static readonly Logger _logger = Logger.GetLogger(typeof(HostEndpoint));
	private readonly string _host;
	private readonly int _port;
	private readonly object _refreshLock = new object();
	private Stopwatch _age;
	private IPEndPoint _endpoint;

	public HostEndpoint(string host, int port)
	{
		_host = host;
		_port = port;
	}

	public TimeSpan RefreshInterval { get; set; } = TimeSpan.FromMinutes(1);

	public bool CacheNegativeResponses { get; set; } = true;

	private bool NeedRefresh => _age == null || _age.Elapsed > RefreshInterval;

	public bool TryGetEndpoint(out IPEndPoint endpoint)
	{
		if (NeedRefresh) Refresh();

		endpoint = _endpoint;
		return endpoint != null;
	}

	public static bool TryParse(string entry, out HostEndpoint hostEndpoint)
	{
		var entries = entry.Split(':');
		if (entries.Length != 2)
		{
			hostEndpoint = null;
			return false;
		}
		if (!int.TryParse(entries[1], out var port))
		{
			hostEndpoint = null;
			return false;
		}
		hostEndpoint = new HostEndpoint(entries[0], port);
		return true;
	}

	private void Refresh()
	{
		lock (_refreshLock)
		{
			_logger.Debug($"Refreshing address for host {_host}");
			try
			{
				var ipEntries = Dns.GetHostAddresses(_host);
				_endpoint = new IPEndPoint(ipEntries.FirstOrDefault(), _port);
			}
			catch (Exception ex)
			{
				_logger.Error(ex, $"Exception while resolving address for host {_host}");
			}
			finally
			{
				if (CacheNegativeResponses || _endpoint != null)
				{
					_age = Stopwatch.StartNew();
					if (_endpoint != null)
						_logger.Debug($"Resolved host {_host} to address {_endpoint.Address} and caching result for {RefreshInterval}");
					else
						_logger.Debug($"Failed to resolve host {_host} and caching negative result for {RefreshInterval}");
				}
			}
		}
	}
}

It's really useful to consider both not permanently caching the resolved address so you don't have to role instances to pick up changes, and caching negative results in case there are problems resolving the hostname which can cause Dns.GetHostAddresses to block for long periods of time which can cause code paths that send segments to x-ray to become very slow.

[emmekappa]

@bittercoder I finished and submitted the PR. I'm also handling just IPV4 addresses and the case in which the address could not be resolved, in this case I'll fallback to the default daemon address which seems the be the default fallback.

[emmekappa]

@bittercoder you're right about both the DNS resolution caching issue and the DNS blocking issue, I'll try to figure out something.

[jaredcnance]

Can you please let us know your use case

This is essential for anyone running Docker on ElasticBeanstalk—or more generally, linked ECS containers.

Has there been any progress on this? Just saw #38

[MrLarssonJr]

With #38 merged I would propose to close this issue.

[yogiraj07]

@bittercoder ,
We have included this feature in 2.5.0 release. Please refer Changelog for more details.

Thanks,
Yogi