Add in-proxy broker reuse metric

- Also fix GetTacticsAppliedReceivers trigger logic which caused broker resets
  on every new tunnel establishment.

psiphon/common/protocol/packed.go

@@ -824,8 +824,9 @@ func init() {
 		{162, "inproxy_dial_webrtc_ice_gathering_duration", intConverter},
 		{163, "inproxy_dial_broker_offer_duration", intConverter},
 		{164, "inproxy_dial_webrtc_connection_duration", intConverter},
+		{165, "inproxy_broker_is_reuse", intConverter},
 
-		// Next key value = 165
+		// Next key value = 166
 	}
 
 	for _, spec := range packedAPIParameterSpecs {

psiphon/config.go

@@ -1660,6 +1660,7 @@ func (config *Config) SetParameters(tag string, skipOnError bool, applyParameter
 	// posting notices.
 
 	config.paramsMutex.Lock()
+	tagUnchanged := tag != "" && tag == config.params.Get().Tag()
 	validationFlags := 0
 	if skipOnError {
 		validationFlags |= parameters.ValidationSkipOnError
@@ -1672,6 +1673,20 @@ func (config *Config) SetParameters(tag string, skipOnError bool, applyParameter
 	p := config.params.Get()
 	config.paramsMutex.Unlock()
 
+	// Skip emitting notices and invoking GetTacticsAppliedReceivers when the
+	// tactics tag is unchanged. The notices are redundant, and the receivers
+	// will unnecessarily reset components such as in-proxy broker clients.
+	//
+	// At this time, the GetTactics call in launchEstablishing can result in
+	// redundant SetParameters calls with an unchanged tag.
+	//
+	// As a fail safe, and since there should not be any unwanted side
+	// effects, the above params.Set is still executed even for unchanged tags.
+
+	if tagUnchanged {
+		return nil
+	}
+
 	NoticeInfo("applied %v parameters with tag '%s'", counts, tag)
 
 	// Emit certain individual parameter values for quick reference in diagnostics.

psiphon/inproxy.go

@@ -148,11 +148,28 @@ func (b *InproxyBrokerClientManager) GetBrokerClient(
 		}
 	}
 
+	// Set isReuse, which will record a metric indicating if this broker
+	// client has already been used for a successful round trip, a case which
+	// should result in faster overall dials.
+	//
+	// Limitations with HasSuccess, and the resulting isReuse metric: in some
+	// cases, it's possible that the underlying TLS connection is still
+	// redialed by net/http; or it's possible that the Noise session is
+	// invalid/expired and must be reestablished; or it can be the case that
+	// a shared broker client is only partially established at this point in
+	// time.
+	//
+	// Return a shallow copy of the broker dial params in order to record the
+	// correct isReuse, which varies depending on previous use.
+
+	brokerDialParams := *b.brokerClientInstance.brokerDialParams
+	brokerDialParams.isReuse = b.brokerClientInstance.HasSuccess()
+
 	// The b.brokerClientInstance.brokerClient is wired up to refer back to
 	// b.brokerClientInstance.brokerDialParams/roundTripper, etc.
 
 	return b.brokerClientInstance.brokerClient,
-		b.brokerClientInstance.brokerDialParams,
+		&brokerDialParams,
 		nil
 }
 
@@ -286,7 +303,6 @@ type InproxyBrokerClientInstance struct {
 	brokerRootObfuscationSecret   inproxy.ObfuscationSecret
 	brokerDialParams              *InproxyBrokerDialParameters
 	replayEnabled                 bool
-	isReplay                      bool
 	roundTripper                  *InproxyBrokerRoundTripper
 	personalCompartmentIDs        []inproxy.ID
 	commonCompartmentIDs          []inproxy.ID
@@ -531,7 +547,6 @@ func NewInproxyBrokerClientInstance(
 		brokerRootObfuscationSecret: brokerRootObfuscationSecret,
 		brokerDialParams:            brokerDialParams,
 		replayEnabled:               replayEnabled,
-		isReplay:                    isReplay,
 		roundTripper:                roundTripper,
 		personalCompartmentIDs:      personalCompartmentIDs,
 		commonCompartmentIDs:        commonCompartmentIDs,
@@ -782,6 +797,15 @@ func prepareInproxyCompartmentIDs(
 	return commonCompartmentIDs, personalCompartmentIDs, nil
 }
 
+// HasSuccess indicates whether this broker client instance has completed at
+// least one successful round trip.
+func (b *InproxyBrokerClientInstance) HasSuccess() bool {
+	b.mutex.Lock()
+	defer b.mutex.Unlock()
+
+	return !b.lastSuccess.IsZero()
+}
+
 // Close closes the broker client round tripped, including closing all
 // underlying network connections, which will interrupt any in-flight round
 // trips.
@@ -856,9 +880,9 @@ func (b *InproxyBrokerClientInstance) BrokerClientRoundTripperSucceeded(roundTri
 	// Set replay or extend the broker dial parameters replay TTL after a
 	// success. With tunnel dial parameters, the replay TTL is extended after
 	// every successful tunnel connection. Since there are potentially more
-	// and more frequent broker round trips one tunnel dial, the TTL is only
-	// extended after some target duration has elapsed, to avoid excessive
-	// datastore writes.
+	// and more frequent broker round trips compared to tunnel dials, the TTL
+	// is only extended after some target duration has elapsed, to avoid
+	// excessive datastore writes.
 
 	if b.replayEnabled && now.Sub(b.lastStoreReplay) > b.replayUpdateFrequency {
 		b.brokerDialParams.LastUsedTimestamp = time.Now()
@@ -1079,6 +1103,7 @@ func (b *InproxyBrokerClientInstance) RelayedPacketRequestTimeout() time.Duratio
 type InproxyBrokerDialParameters struct {
 	brokerSpec *parameters.InproxyBrokerSpec `json:"-"`
 	isReplay   bool                          `json:"-"`
+	isReuse    bool                          `json:"-"`
 
 	LastUsedTimestamp      time.Time
 	LastUsedBrokerSpecHash []byte
@@ -1152,6 +1177,9 @@ func (brokerDialParams *InproxyBrokerDialParameters) prepareDialConfigs(
 
 	brokerDialParams.isReplay = isReplay
 
+	// brokerDialParams.isReuse is set only later, as this is a new broker
+	// client dial.
+
 	if isReplay {
 		// FrontedHTTPDialParameters
 		//
@@ -1206,6 +1234,12 @@ func (brokerDialParams *InproxyBrokerDialParameters) GetMetrics() common.LogFiel
 	}
 	logFields["inproxy_broker_is_replay"] = isReplay
 
+	isReuse := "0"
+	if brokerDialParams.isReuse {
+		isReuse = "1"
+	}
+	logFields["inproxy_broker_is_reuse"] = isReuse
+
 	return logFields
 }
 

psiphon/server/api.go

@@ -1197,6 +1197,7 @@ var inproxyDialParams = []requestParamSpec{
 	{"inproxy_dial_webrtc_ice_gathering_duration", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_dial_broker_offer_duration", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_dial_webrtc_connection_duration", isIntString, requestParamOptional | requestParamLogStringAsInt},
+	{"inproxy_broker_is_reuse", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 }
 
 // baseAndDialParams adds baseDialParams and inproxyDialParams to baseParams.

psiphon/server/server_test.go

@@ -2537,6 +2537,8 @@ func checkExpectedServerTunnelLogFields(
 
 			// Fields sent by the client
 
+			"inproxy_broker_is_replay",
+			"inproxy_broker_is_reuse",
 			"inproxy_broker_transport",
 			"inproxy_broker_fronting_provider_id",
 			"inproxy_broker_dial_address",