Validate and log tactics API parameters

- Refactor dialStats initialization for use
  in tactics request API parameters.

- Validation and logging of API parameters
  in tactics request.

- Translate speed test sample field names
  to verbose, readable values when logging
  metrics.

psiphon/common/tactics/tactics.go

@@ -344,13 +344,18 @@ type Tactics struct {
 	Parameters map[string]interface{}
 }
 
+// Note: the SpeedTestSample json tags are selected to minimize marshaled
+// size. In psiphond, for logging metrics, the field names are translated to
+// more verbose values. psiphon/server.makeSpeedTestSamplesLogField currently
+// hard-codes these same SpeedTestSample json tag values for that translation.
+
 // SpeedTestSample is speed test data for a single RTT event.
 type SpeedTestSample struct {
 
 	// Timestamp is the speed test event time, and may be used to discard
 	// stale samples. The server supplies the speed test timestamp. This
 	// value is truncated to the nearest hour as a privacy measure.
-	Timestamp time.Time `json:"t"`
+	Timestamp time.Time `json:"s"`
 
 	// EndPointRegion is the region of the endpoint, the Psiphon server,
 	// used for the speed test. This may be used to exclude outlier samples
@@ -367,15 +372,15 @@ type SpeedTestSample struct {
 	// TCP, TLS, or SSH handshakes.
 	// This value is truncated to the nearest millisecond as a privacy
 	// measure.
-	RTTMilliseconds int `json:"rtt"`
+	RTTMilliseconds int `json:"t"`
 
 	// BytesUp is the size of the upstream payload in the round trip.
 	// Currently, the payload is limited to anti-fingerprint padding.
-	BytesUp int `json:"up"`
+	BytesUp int `json:"u"`
 
 	// BytesDown is the size of the downstream payload in the round trip.
 	// Currently, the payload is limited to anti-fingerprint padding.
-	BytesDown int `json:"dn"`
+	BytesDown int `json:"d"`
 }
 
 // GenerateKeys generates a tactics request key pair and obfuscation key.
@@ -951,16 +956,13 @@ func (server *Server) handleTacticsRequest(
 		return
 	}
 
-	// *TODO* fix validator and formatter
-	/*
-		err = server.apiParameterValidator(apiParams)
-		if err != nil {
-			server.logger.WithContextFields(
-				common.LogFields{"error": err}).Warning("invalid request parameters")
-			w.WriteHeader(http.StatusNotFound)
-			return
-		}
-	*/
+	err = server.apiParameterValidator(apiParams)
+	if err != nil {
+		server.logger.WithContextFields(
+			common.LogFields{"error": err}).Warning("invalid request parameters")
+		w.WriteHeader(http.StatusNotFound)
+		return
+	}
 
 	tacticsPayload, err := server.GetTacticsPayload(geoIPData, apiParams)
 	if err != nil {
@@ -991,11 +993,7 @@ func (server *Server) handleTacticsRequest(
 
 	// Log a metric.
 
-	// *TODO* fix validator and formatter
-	/*
-		logFields := server.logFieldFormatter(geoIPData, apiParams)
-	*/
-	logFields := make(common.LogFields)
+	logFields := server.logFieldFormatter(geoIPData, apiParams)
 
 	logFields[NEW_TACTICS_TAG_LOG_FIELD_NAME] = tacticsPayload.Tag
 	logFields[IS_TACTICS_REQUEST_LOG_FIELD_NAME] = true

psiphon/controller.go

@@ -1378,8 +1378,13 @@ func (controller *Controller) doFetchTactics(
 
 	meekConfig.RoundTripperOnly = true
 
-	dialConfig, _, _ := initDialConfig(
-		controller.config, meekConfig)
+	dialConfig, dialStats := initDialConfig(controller.config, meekConfig)
+
+	NoticeRequestingTactics(
+		serverEntry.IpAddress,
+		serverEntry.Region,
+		tacticsProtocol,
+		dialStats)
 
 	// TacticsTimeout should be a very long timeout, since it's not
 	// adjusted by tactics in a new network context, and so clients
@@ -1421,9 +1426,12 @@ func (controller *Controller) doFetchTactics(
 	}
 	defer meekConn.Close()
 
-	var apiParams common.APIParameters
-	// *TODO* populate
-	apiParams = make(common.APIParameters)
+	apiParams := getBaseAPIParameters(
+		controller.config,
+		controller.sessionId,
+		serverEntry,
+		tacticsProtocol,
+		dialStats)
 
 	tacticsRecord, err := tactics.FetchTactics(
 		ctx,
@@ -1440,6 +1448,12 @@ func (controller *Controller) doFetchTactics(
 		return nil, common.ContextError(err)
 	}
 
+	NoticeRequestedTactics(
+		serverEntry.IpAddress,
+		serverEntry.Region,
+		tacticsProtocol,
+		dialStats)
+
 	return tacticsRecord, nil
 }
 

psiphon/notice.go

@@ -392,7 +392,7 @@ func NoticeAvailableEgressRegions(regions []string) {
 		"AvailableEgressRegions", 0, "regions", sortedRegions)
 }
 
-func noticeServerDialStats(noticeType, ipAddress, region, protocol string, tunnelDialStats *TunnelDialStats) {
+func noticeWithDialStats(noticeType, ipAddress, region, protocol string, dialStats *DialStats) {
 
 	args := []interface{}{
 		"ipAddress", ipAddress,
@@ -400,45 +400,46 @@ func noticeServerDialStats(noticeType, ipAddress, region, protocol string, tunne
 		"protocol", protocol,
 	}
 
-	if tunnelDialStats.SelectedSSHClientVersion {
-		args = append(args, "SSHClientVersion", tunnelDialStats.SSHClientVersion)
+	if dialStats.SelectedSSHClientVersion {
+		args = append(args, "SSHClientVersion", dialStats.SSHClientVersion)
 	}
 
-	if tunnelDialStats.UpstreamProxyType != "" {
-		args = append(args, "upstreamProxyType", tunnelDialStats.UpstreamProxyType)
+	if dialStats.UpstreamProxyType != "" {
+		args = append(args, "upstreamProxyType", dialStats.UpstreamProxyType)
 	}
 
-	if tunnelDialStats.UpstreamProxyCustomHeaderNames != nil {
-		args = append(args, "upstreamProxyCustomHeaderNames", strings.Join(tunnelDialStats.UpstreamProxyCustomHeaderNames, ","))
+	if dialStats.UpstreamProxyCustomHeaderNames != nil {
+		args = append(args, "upstreamProxyCustomHeaderNames", strings.Join(dialStats.UpstreamProxyCustomHeaderNames, ","))
 	}
 
-	if tunnelDialStats.MeekDialAddress != "" {
-		args = append(args, "meekDialAddress", tunnelDialStats.MeekDialAddress)
+	if dialStats.MeekDialAddress != "" {
+		args = append(args, "meekDialAddress", dialStats.MeekDialAddress)
 	}
 
-	if tunnelDialStats.MeekResolvedIPAddress != "" {
-		args = append(args, "meekResolvedIPAddress", tunnelDialStats.MeekResolvedIPAddress)
+	meekResolvedIPAddress := dialStats.MeekResolvedIPAddress.Load().(string)
+	if meekResolvedIPAddress != "" {
+		args = append(args, "meekResolvedIPAddress", meekResolvedIPAddress)
 	}
 
-	if tunnelDialStats.MeekSNIServerName != "" {
-		args = append(args, "meekSNIServerName", tunnelDialStats.MeekSNIServerName)
+	if dialStats.MeekSNIServerName != "" {
+		args = append(args, "meekSNIServerName", dialStats.MeekSNIServerName)
 	}
 
-	if tunnelDialStats.MeekHostHeader != "" {
-		args = append(args, "meekHostHeader", tunnelDialStats.MeekHostHeader)
+	if dialStats.MeekHostHeader != "" {
+		args = append(args, "meekHostHeader", dialStats.MeekHostHeader)
 	}
 
 	// MeekTransformedHostName is meaningful when meek is used, which is when MeekDialAddress != ""
-	if tunnelDialStats.MeekDialAddress != "" {
-		args = append(args, "meekTransformedHostName", tunnelDialStats.MeekTransformedHostName)
+	if dialStats.MeekDialAddress != "" {
+		args = append(args, "meekTransformedHostName", dialStats.MeekTransformedHostName)
 	}
 
-	if tunnelDialStats.SelectedUserAgent {
-		args = append(args, "userAgent", tunnelDialStats.UserAgent)
+	if dialStats.SelectedUserAgent {
+		args = append(args, "userAgent", dialStats.UserAgent)
 	}
 
-	if tunnelDialStats.SelectedTLSProfile {
-		args = append(args, "TLSProfile", tunnelDialStats.TLSProfile)
+	if dialStats.SelectedTLSProfile {
+		args = append(args, "TLSProfile", dialStats.TLSProfile)
 	}
 
 	singletonNoticeLogger.outputNotice(
@@ -447,15 +448,27 @@ func noticeServerDialStats(noticeType, ipAddress, region, protocol string, tunne
 }
 
 // NoticeConnectingServer reports parameters and details for a single connection attempt
-func NoticeConnectingServer(ipAddress, region, protocol string, tunnelDialStats *TunnelDialStats) {
-	noticeServerDialStats(
-		"ConnectingServer", ipAddress, region, protocol, tunnelDialStats)
+func NoticeConnectingServer(ipAddress, region, protocol string, dialStats *DialStats) {
+	noticeWithDialStats(
+		"ConnectingServer", ipAddress, region, protocol, dialStats)
 }
 
 // NoticeConnectedServer reports parameters and details for a single successful connection
-func NoticeConnectedServer(ipAddress, region, protocol string, tunnelDialStats *TunnelDialStats) {
-	noticeServerDialStats(
-		"ConnectedServer", ipAddress, region, protocol, tunnelDialStats)
+func NoticeConnectedServer(ipAddress, region, protocol string, dialStats *DialStats) {
+	noticeWithDialStats(
+		"ConnectedServer", ipAddress, region, protocol, dialStats)
+}
+
+// NoticeRequestingTactics reports parameters and details for a tactics request attempt
+func NoticeRequestingTactics(ipAddress, region, protocol string, dialStats *DialStats) {
+	noticeWithDialStats(
+		"RequestingTactics", ipAddress, region, protocol, dialStats)
+}
+
+// NoticeRequestedTactics reports parameters and details for a successful tactics request
+func NoticeRequestedTactics(ipAddress, region, protocol string, dialStats *DialStats) {
+	noticeWithDialStats(
+		"RequestedTactics", ipAddress, region, protocol, dialStats)
 }
 
 // NoticeActiveTunnel is a successful connection that is used as an active tunnel for port forwarding

psiphon/server/api.go

@@ -177,9 +177,7 @@ func dispatchAPIRequestHandler(
 }
 
 var handshakeRequestParams = append(
-	[]requestParamSpec{
-		{tactics.STORED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
-		{tactics.SPEED_TEST_SAMPLES_PARAMETER_NAME, nil, requestParamOptional | requestParamJSON}},
+	append([]requestParamSpec(nil), tacticsParams...),
 	baseRequestParams...)
 
 // handshakeAPIRequestHandler implements the "handshake" API request.
@@ -530,9 +528,18 @@ func clientVerificationAPIRequestHandler(
 	}
 }
 
+var tacticsParams = []requestParamSpec{
+	{tactics.STORED_TACTICS_TAG_PARAMETER_NAME, isAnyString, requestParamOptional},
+	{tactics.SPEED_TEST_SAMPLES_PARAMETER_NAME, nil, requestParamOptional | requestParamJSON},
+}
+
+var tacticsRequestParams = append(
+	append([]requestParamSpec(nil), tacticsParams...),
+	baseRequestParams...)
+
 func getTacticsAPIParameterValidator(config *Config) common.APIParameterValidator {
 	return func(params common.APIParameters) error {
-		return validateRequestParams(config, params, handshakeRequestParams)
+		return validateRequestParams(config, params, tacticsRequestParams)
 	}
 }
 
@@ -545,7 +552,7 @@ func getTacticsAPIParameterLogFieldFormatter() common.APIParameterLogFieldFormat
 			GeoIPData(geoIPData),
 			nil, // authorizedAccessTypes are not known yet
 			params,
-			handshakeRequestParams)
+			tacticsRequestParams)
 
 		return common.LogFields(logFields)
 	}
@@ -765,19 +772,54 @@ func getRequestLogFields(
 			}
 
 		case []interface{}:
-			// Note: actually validated as an array of strings
-			logFields[expectedParam.name] = v
+			if expectedParam.name == tactics.SPEED_TEST_SAMPLES_PARAMETER_NAME {
+				logFields[expectedParam.name] = makeSpeedTestSamplesLogField(v)
+			} else {
+				logFields[expectedParam.name] = v
+			}
 
 		default:
-			// This type assertion should be checked already in
-			// validateRequestParams, so failure is unexpected.
-			continue
+			logFields[expectedParam.name] = v
 		}
 	}
 
 	return logFields
 }
 
+// makeSpeedTestSamplesLogField renames the tactics.SpeedTestSample json tag
+// fields to more verbose names for metrics.
+func makeSpeedTestSamplesLogField(samples []interface{}) []interface{} {
+	// TODO: use reflection and add additional tags, e.g.,
+	// `json:"s" log:"timestamp"` to remove hard-coded
+	// tag value dependency?
+	logSamples := make([]interface{}, len(samples))
+	for i, sample := range samples {
+		logSample := make(map[string]interface{})
+		if m, ok := sample.(map[string]interface{}); ok {
+			for k, v := range m {
+				logK := k
+				switch k {
+				case "s":
+					logK = "timestamp"
+				case "r":
+					logK = "server_region"
+				case "p":
+					logK = "relay_protocol"
+				case "t":
+					logK = "round_trip_time_ms"
+				case "u":
+					logK = "bytes_up"
+				case "d":
+					logK = "bytes_down"
+				}
+				logSample[logK] = v
+			}
+		}
+		logSamples[i] = logSample
+	}
+	return logSamples
+}
+
 func getStringRequestParam(params common.APIParameters, name string) (string, error) {
 	if params[name] == nil {
 		return "", common.ContextError(fmt.Errorf("missing param: %s", name))

psiphon/serverApi.go

@@ -123,7 +123,7 @@ func NewServerContext(tunnel *Tunnel) (*ServerContext, error) {
 func (serverContext *ServerContext) doHandshakeRequest(
 	ignoreStatsRegexps bool) error {
 
-	params := serverContext.getBaseParams()
+	params := serverContext.getBaseAPIParameters()
 
 	doTactics := serverContext.tunnel.config.NetworkIDGetter != nil
 	networkID := ""
@@ -295,7 +295,7 @@ func (serverContext *ServerContext) doHandshakeRequest(
 // a unique user for a time period.
 func (serverContext *ServerContext) DoConnectedRequest() error {
 
-	params := serverContext.getBaseParams()
+	params := serverContext.getBaseAPIParameters()
 
 	lastConnected, err := GetKeyValue(DATA_STORE_LAST_CONNECTED_KEY)
 	if err != nil {
@@ -417,7 +417,7 @@ func (serverContext *ServerContext) DoStatusRequest(tunnel *Tunnel) error {
 func (serverContext *ServerContext) getStatusParams(
 	isTunneled bool) common.APIParameters {
 
-	params := serverContext.getBaseParams()
+	params := serverContext.getBaseAPIParameters()
 
 	// Add a random amount of padding to help prevent stats updates from being
 	// a predictable size (which often happens when the connection is quiet).
@@ -600,7 +600,7 @@ func RecordRemoteServerListStat(
 func (serverContext *ServerContext) DoClientVerificationRequest(
 	verificationPayload string, serverIP string) error {
 
-	params := serverContext.getBaseParams()
+	params := serverContext.getBaseAPIParameters()
 	var response []byte
 	var err error
 
@@ -721,97 +721,109 @@ func (serverContext *ServerContext) doPostRequest(
 	return responseBody, nil
 }
 
-// getBaseParams returns all the common API parameters that are included
-// with each Psiphon API request. These common parameters are used for
-// statistics.
-func (serverContext *ServerContext) getBaseParams() common.APIParameters {
+func (serverContext *ServerContext) getBaseAPIParameters() common.APIParameters {
+	return getBaseAPIParameters(
+		serverContext.tunnel.config,
+		serverContext.sessionId,
+		serverContext.tunnel.serverEntry,
+		serverContext.tunnel.protocol,
+		serverContext.tunnel.dialStats)
+}
+
+// getBaseAPIParameters returns all the common API parameters that are
+// included with each Psiphon API request. These common parameters are used
+// for metrics.
+func getBaseAPIParameters(
+	config *Config,
+	sessionID string,
+	serverEntry *protocol.ServerEntry,
+	protocol string,
+	dialStats *DialStats) common.APIParameters {
 
 	params := make(common.APIParameters)
 
-	tunnel := serverContext.tunnel
-
-	params["session_id"] = serverContext.sessionId
-	params["client_session_id"] = serverContext.sessionId
-	params["server_secret"] = tunnel.serverEntry.WebServerSecret
-	params["propagation_channel_id"] = tunnel.config.PropagationChannelId
-	params["sponsor_id"] = tunnel.config.SponsorId
-	params["client_version"] = tunnel.config.ClientVersion
-	// TODO: client_tunnel_core_version?
-	params["relay_protocol"] = tunnel.protocol
-	params["client_platform"] = tunnel.config.ClientPlatform
+	params["session_id"] = sessionID
+	params["client_session_id"] = sessionID
+	params["server_secret"] = serverEntry.WebServerSecret
+	params["propagation_channel_id"] = config.PropagationChannelId
+	params["sponsor_id"] = config.SponsorId
+	params["client_version"] = config.ClientVersion
+	params["relay_protocol"] = protocol
+	params["client_platform"] = config.ClientPlatform
 	params["client_build_rev"] = common.GetBuildInfo().BuildRev
-	params["tunnel_whole_device"] = strconv.Itoa(tunnel.config.TunnelWholeDevice)
+	params["tunnel_whole_device"] = strconv.Itoa(config.TunnelWholeDevice)
 
 	// The following parameters may be blank and must
 	// not be sent to the server if blank.
 
-	if tunnel.config.DeviceRegion != "" {
-		params["device_region"] = tunnel.config.DeviceRegion
+	if config.DeviceRegion != "" {
+		params["device_region"] = config.DeviceRegion
 	}
 
-	if tunnel.dialStats.SelectedSSHClientVersion {
-		params["ssh_client_version"] = tunnel.dialStats.SSHClientVersion
+	if dialStats.SelectedSSHClientVersion {
+		params["ssh_client_version"] = dialStats.SSHClientVersion
 	}
 
-	if tunnel.dialStats.UpstreamProxyType != "" {
-		params["upstream_proxy_type"] = tunnel.dialStats.UpstreamProxyType
+	if dialStats.UpstreamProxyType != "" {
+		params["upstream_proxy_type"] = dialStats.UpstreamProxyType
 	}
 
-	if tunnel.dialStats.UpstreamProxyCustomHeaderNames != nil {
-		params["upstream_proxy_custom_header_names"] = tunnel.dialStats.UpstreamProxyCustomHeaderNames
+	if dialStats.UpstreamProxyCustomHeaderNames != nil {
+		params["upstream_proxy_custom_header_names"] = dialStats.UpstreamProxyCustomHeaderNames
 	}
 
-	if tunnel.dialStats.MeekDialAddress != "" {
-		params["meek_dial_address"] = tunnel.dialStats.MeekDialAddress
+	if dialStats.MeekDialAddress != "" {
+		params["meek_dial_address"] = dialStats.MeekDialAddress
 	}
 
-	if tunnel.dialStats.MeekResolvedIPAddress != "" {
-		params["meek_resolved_ip_address"] = tunnel.dialStats.MeekResolvedIPAddress
+	meekResolvedIPAddress := dialStats.MeekResolvedIPAddress.Load().(string)
+	if meekResolvedIPAddress != "" {
+		params["meek_resolved_ip_address"] = meekResolvedIPAddress
 	}
 
-	if tunnel.dialStats.MeekSNIServerName != "" {
-		params["meek_sni_server_name"] = tunnel.dialStats.MeekSNIServerName
+	if dialStats.MeekSNIServerName != "" {
+		params["meek_sni_server_name"] = dialStats.MeekSNIServerName
 	}
 
-	if tunnel.dialStats.MeekHostHeader != "" {
-		params["meek_host_header"] = tunnel.dialStats.MeekHostHeader
+	if dialStats.MeekHostHeader != "" {
+		params["meek_host_header"] = dialStats.MeekHostHeader
 	}
 
 	// MeekTransformedHostName is meaningful when meek is used, which is when MeekDialAddress != ""
-	if tunnel.dialStats.MeekDialAddress != "" {
+	if dialStats.MeekDialAddress != "" {
 		transformedHostName := "0"
-		if tunnel.dialStats.MeekTransformedHostName {
+		if dialStats.MeekTransformedHostName {
 			transformedHostName = "1"
 		}
 		params["meek_transformed_host_name"] = transformedHostName
 	}
 
-	if tunnel.dialStats.SelectedUserAgent {
-		params["user_agent"] = tunnel.dialStats.UserAgent
+	if dialStats.SelectedUserAgent {
+		params["user_agent"] = dialStats.UserAgent
 	}
 
-	if tunnel.dialStats.SelectedTLSProfile {
-		params["tls_profile"] = tunnel.dialStats.TLSProfile
+	if dialStats.SelectedTLSProfile {
+		params["tls_profile"] = dialStats.TLSProfile
 	}
 
-	if tunnel.serverEntry.Region != "" {
-		params["server_entry_region"] = tunnel.serverEntry.Region
+	if serverEntry.Region != "" {
+		params["server_entry_region"] = serverEntry.Region
 	}
 
-	if tunnel.serverEntry.LocalSource != "" {
-		params["server_entry_source"] = tunnel.serverEntry.LocalSource
+	if serverEntry.LocalSource != "" {
+		params["server_entry_source"] = serverEntry.LocalSource
 	}
 
 	// As with last_connected, this timestamp stat, which may be
 	// a precise handshake request server timestamp, is truncated
 	// to hour granularity to avoid introducing a reconstructable
 	// cross-session user trace into server logs.
-	localServerEntryTimestamp := common.TruncateTimestampToHour(tunnel.serverEntry.LocalTimestamp)
+	localServerEntryTimestamp := common.TruncateTimestampToHour(serverEntry.LocalTimestamp)
 	if localServerEntryTimestamp != "" {
 		params["server_entry_timestamp"] = localServerEntryTimestamp
 	}
 
-	params[tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME] = tunnel.config.clientParameters.Get().Tag()
+	params[tactics.APPLIED_TACTICS_TAG_PARAMETER_NAME] = config.clientParameters.Get().Tag()
 
 	return params
 }

psiphon/tunnel.go

@@ -98,23 +98,29 @@ type Tunnel struct {
 	adjustedEstablishStartTime   monotime.Time
 	establishDuration            time.Duration
 	establishedTime              monotime.Time
-	dialStats                    *TunnelDialStats
+	dialStats                    *DialStats
 	newClientVerificationPayload chan string
 }
 
-// TunnelDialStats records additional dial config that is sent to the server for stats
-// recording. This data is used to analyze which configuration settings are successful
-// in various circumvention contexts, and includes meek dial params and upstream proxy
-// params.
-// For upstream proxy, only proxy type and custom header names are recorded; proxy
-// address and custom header values are considered PII.
-type TunnelDialStats struct {
+// DialStats records additional dial config that is sent to the server for
+// stats recording. This data is used to analyze which configuration settings
+// are successful in various circumvention contexts, and includes meek dial
+// params and upstream proxy params.
+//
+// For upstream proxy, only proxy type and custom header names are recorded;
+// proxy address and custom header values are considered PII.
+//
+// MeekResolvedIPAddress is set asynchronously, as it is not known until the
+// dial process has begun. The atomic.Value will contain a string, initialized
+// to "", and set to the resolved IP address once that part of the dial
+// process has completed.
+type DialStats struct {
 	SelectedSSHClientVersion       bool
 	SSHClientVersion               string
 	UpstreamProxyType              string
 	UpstreamProxyCustomHeaderNames []string
 	MeekDialAddress                string
-	MeekResolvedIPAddress          string
+	MeekResolvedIPAddress          atomic.Value
 	MeekSNIServerName              string
 	MeekHostHeader                 string
 	MeekTransformedHostName        bool
@@ -748,8 +754,7 @@ func initMeekConfig(
 
 // initDialConfig is a helper that creates a DialConfig for the tunnel.
 func initDialConfig(
-	config *Config,
-	meekConfig *MeekConfig) (*DialConfig, string, bool) {
+	config *Config, meekConfig *MeekConfig) (*DialConfig, *DialStats) {
 
 	var upstreamProxyType string
 
@@ -786,18 +791,62 @@ func initDialConfig(
 		selectedUserAgent = UserAgentIfUnset(config.clientParameters, dialCustomHeaders)
 	}
 
-	return &DialConfig{
-			UpstreamProxyURL:              config.UpstreamProxyURL,
-			CustomHeaders:                 dialCustomHeaders,
-			DeviceBinder:                  config.DeviceBinder,
-			DnsServerGetter:               config.DnsServerGetter,
-			IPv6Synthesizer:               config.IPv6Synthesizer,
-			UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
-			TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
-			DeviceRegion:                  config.DeviceRegion,
-		},
-		upstreamProxyType,
-		selectedUserAgent
+	dialConfig := &DialConfig{
+		UpstreamProxyURL:              config.UpstreamProxyURL,
+		CustomHeaders:                 dialCustomHeaders,
+		DeviceBinder:                  config.DeviceBinder,
+		DnsServerGetter:               config.DnsServerGetter,
+		IPv6Synthesizer:               config.IPv6Synthesizer,
+		UseIndistinguishableTLS:       config.UseIndistinguishableTLS,
+		TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
+		DeviceRegion:                  config.DeviceRegion,
+	}
+
+	dialStats := &DialStats{}
+
+	if selectedUserAgent {
+		dialStats.SelectedUserAgent = true
+		dialStats.UserAgent = dialConfig.CustomHeaders.Get("User-Agent")
+	}
+
+	if upstreamProxyType != "" {
+		dialStats.UpstreamProxyType = upstreamProxyType
+	}
+
+	if len(dialConfig.CustomHeaders) > 0 {
+		dialStats.UpstreamProxyCustomHeaderNames = make([]string, 0)
+		for name := range dialConfig.CustomHeaders {
+			if selectedUserAgent && name == "User-Agent" {
+				continue
+			}
+			dialStats.UpstreamProxyCustomHeaderNames = append(dialStats.UpstreamProxyCustomHeaderNames, name)
+		}
+	}
+
+	// Unconditionally initialize MeekResolvedIPAddress, so a valid string can
+	// always be read.
+	dialStats.MeekResolvedIPAddress.Store("")
+
+	if meekConfig != nil {
+		dialStats.MeekDialAddress = meekConfig.DialAddress
+		dialStats.MeekSNIServerName = meekConfig.SNIServerName
+		dialStats.MeekHostHeader = meekConfig.HostHeader
+		dialStats.MeekTransformedHostName = meekConfig.TransformedHostName
+		dialStats.SelectedTLSProfile = true
+		dialStats.TLSProfile = meekConfig.TLSProfile
+
+		// Use an asynchronous callback to record the resolved IP address when
+		// dialing a domain name. Note that DialMeek doesn't immediately
+		// establish any HTTP connections, so the resolved IP address won't be
+		// reported in all cases until after SSH traffic is relayed or a
+		// endpoint request is made over the meek connection.
+		dialConfig.ResolvedIPCallback = func(IPAddress string) {
+			dialStats.MeekResolvedIPAddress.Store(IPAddress)
+		}
+
+	}
+
+	return dialConfig, dialStats
 }
 
 type dialResult struct {
@@ -805,7 +854,7 @@ type dialResult struct {
 	monitoredConn *common.ActivityMonitoredConn
 	sshClient     *ssh.Client
 	sshRequests   <-chan *ssh.Request
-	dialStats     *TunnelDialStats
+	dialStats     *DialStats
 }
 
 // dialSsh is a helper that builds the transport layers and establishes the SSH connection.
@@ -858,59 +907,17 @@ func dialSsh(
 		}
 	}
 
-	dialConfig, upstreamProxyType, selectedUserAgent := initDialConfig(config, meekConfig)
-
-	// Use an asynchronous callback to record the resolved IP address when
-	// dialing a domain name. Note that DialMeek doesn't immediately
-	// establish any HTTPS connections, so the resolved IP address won't be
-	// reported until during/after ssh session establishment (the ssh traffic
-	// is meek payload). So don't Load() the IP address value until after that
-	// has completed to ensure a result.
-	var resolvedIPAddress atomic.Value
-	resolvedIPAddress.Store("")
-	dialConfig.ResolvedIPCallback = func(IPAddress string) {
-		resolvedIPAddress.Store(IPAddress)
-	}
-
-	// Gather dial parameters for diagnostic logging and stats reporting
+	dialConfig, dialStats := initDialConfig(config, meekConfig)
 
-	dialStats := &TunnelDialStats{}
+	// Add dial stats specific to SSH dialing
 
 	if selectedSSHClientVersion {
 		dialStats.SelectedSSHClientVersion = true
 		dialStats.SSHClientVersion = SSHClientVersion
 	}
 
-	if selectedUserAgent {
-		dialStats.SelectedUserAgent = true
-		dialStats.UserAgent = dialConfig.CustomHeaders.Get("User-Agent")
-	}
-
-	if upstreamProxyType != "" {
-		dialStats.UpstreamProxyType = upstreamProxyType
-	}
-
-	if len(dialConfig.CustomHeaders) > 0 {
-		dialStats.UpstreamProxyCustomHeaderNames = make([]string, 0)
-		for name := range dialConfig.CustomHeaders {
-			if selectedUserAgent && name == "User-Agent" {
-				continue
-			}
-			dialStats.UpstreamProxyCustomHeaderNames = append(dialStats.UpstreamProxyCustomHeaderNames, name)
-		}
-	}
-
-	if meekConfig != nil {
-		// Note: dialStats.MeekResolvedIPAddress isn't set until the dial begins,
-		// so it will always be blank in NoticeConnectingServer.
-		dialStats.MeekDialAddress = meekConfig.DialAddress
-		dialStats.MeekResolvedIPAddress = ""
-		dialStats.MeekSNIServerName = meekConfig.SNIServerName
-		dialStats.MeekHostHeader = meekConfig.HostHeader
-		dialStats.MeekTransformedHostName = meekConfig.TransformedHostName
-		dialStats.SelectedTLSProfile = true
-		dialStats.TLSProfile = meekConfig.TLSProfile
-	}
+	// Note: dialStats.MeekResolvedIPAddress isn't set until the dial begins,
+	// so it will always be blank in NoticeConnectingServer.
 
 	NoticeConnectingServer(
 		serverEntry.IpAddress,
@@ -1063,12 +1070,6 @@ func dialSsh(
 		return nil, common.ContextError(result.err)
 	}
 
-	// Update dial parameters determined during dial
-
-	if dialStats != nil && meekConfig != nil {
-		dialStats.MeekResolvedIPAddress = resolvedIPAddress.Load().(string)
-	}
-
 	NoticeConnectedServer(
 		serverEntry.IpAddress,
 		serverEntry.Region,