Skip to content

Conversation

@panavenue
Copy link
Collaborator

issue: #936

Instead of using the original string() method which contains the dns name. Update it to follow the standard icn name standard.

@panavenue panavenue marked this pull request as ready for review August 20, 2025 18:41
@panavenue panavenue requested a review from a team as a code owner August 20, 2025 18:41
@panavenue panavenue requested review from hessjcg and kgala2 August 20, 2025 18:43
@hessjcg
Copy link
Collaborator

hessjcg commented Aug 25, 2025

Maybe we could do something like this:

	// Log if resolver changed the instance name input string.
	// icn = "pro:reg:inst" -> cn = "pro:reg:inst" cn.String() = "pro:reg:inst"
	// icn = "example.com" -> cn = "example.com", "pro:reg:inst" cn.String() = "example.com -> pro:reg:inst"
	// icn = "my-custom-name" -> cn = "pro:reg:inst" cn.String() = "pro:reg:inst"
	if cn.DomainName != "" {
	    // icn is a domain name
		d.logger.Debugf(ctx, "resolved instance domain name %s to %s", icn, ... pro:reg:inst)
	}
	else if cn.String() != icn {
	    // icn was not a domain name, but the resolver changed it and cn != icn
		d.logger.Debugf(ctx, "resolved instance connection string %s to %s", icn, cn)
	}

@panavenue
Copy link
Collaborator Author

Maybe we could do something like this:

	// Log if resolver changed the instance name input string.
	// icn = "pro:reg:inst" -> cn = "pro:reg:inst" cn.String() = "pro:reg:inst"
	// icn = "example.com" -> cn = "example.com", "pro:reg:inst" cn.String() = "example.com -> pro:reg:inst"
	// icn = "my-custom-name" -> cn = "pro:reg:inst" cn.String() = "pro:reg:inst"
	if cn.DomainName != "" {
	    // icn is a domain name
		d.logger.Debugf(ctx, "resolved instance domain name %s to %s", icn, ... pro:reg:inst)
	}
	else if cn.String() != icn {
	    // icn was not a domain name, but the resolver changed it and cn != icn
		d.logger.Debugf(ctx, "resolved instance connection string %s to %s", icn, cn)
	}

Good call! I think I get the logic now.

I modified a bit for the cn.DomainName != "" part, because cn.String() already in a correct format of "DomainName -> ICN"

Copy link
Collaborator

@hessjcg hessjcg left a comment

Choose a reason for hiding this comment

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

Approved with the small log message change.

@panavenue panavenue merged commit 2832e33 into main Sep 10, 2025
15 checks passed
@panavenue panavenue deleted the fix-log-message branch September 10, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants