review feedback

Author: davidhewitt

logfire/_internal/config.py

@@ -936,10 +936,14 @@ def check_token():
 
                     base_url = self.advanced.generate_base_url(self.token)
                     headers['authorization'] = self.token
-                elif self.send_to_logfire is True and self.advanced.base_url is not None:
-                    # We may not need a token if we are sending to a custom
-                    # base URL
-                    base_url = self.advanced.base_url
+                elif (
+                    self.send_to_logfire is True
+                    and (provided_base_url := self.advanced.base_url) is not None
+                    and provided_base_url.startswith('grpc')
+                ):
+                    # We may not need a token if we are sending to a grpc
+                    # endpoint; it could be an otel collector acting as a proxy
+                    base_url = provided_base_url
 
                 if base_url is not None:
                     if base_url.startswith('grpc://'):
@@ -961,9 +965,6 @@ def check_token():
                             endpoint=base_url,
                             headers=headers,
                             compression=GrpcCompression.Gzip,
-                            # I'm pretty sure that this line here is redundant,
-                            # and that passing it to the QuietMetricExporter is what matters
-                            # because the PeriodicExportingMetricReader will read it from there.
                             preferred_temporality=METRICS_PREFERRED_TEMPORALITY,
                         )
                         log_exporter = GrpcOTLPLogExporter(
@@ -984,9 +985,6 @@ def check_token():
                             headers=headers,
                             session=session,
                             compression=Compression.Gzip,
-                            # I'm pretty sure that this line here is redundant,
-                            # and that passing it to the QuietMetricExporter is what matters
-                            # because the PeriodicExportingMetricReader will read it from there.
                             preferred_temporality=METRICS_PREFERRED_TEMPORALITY,
                         )
                         log_exporter = OTLPLogExporter(
@@ -1020,6 +1018,11 @@ def check_token():
                             PeriodicExportingMetricReader(
                                 QuietMetricExporter(
                                     metric_exporter,
+                                    # NB this could really be retrieved from `metric_exporter` by `QuietMetricExporter`,
+                                    # but it is currently a private attribute on `MetricExporter`, we preferred not to reach
+                                    # inside the otel SDK details.
+                                    #
+                                    # Just make sure it always matches.
                                     preferred_temporality=METRICS_PREFERRED_TEMPORALITY,
                                 )
                             )