Skip to content

Commit 07a3afe

Browse files
allanrenucciGoogle-ML-Automation
authored andcommitted
Unify OSS and internal logging libraries.
This removes the duplicated implementation of `LOG`, `VLOG` and `CHECK` macros and relies directly on the implementation provided by Abseil. TSL logging is not consistently used in XLA. Sometime the Abseil logging headers are included instead. This is not a problem internally because TSL logging aliases Abseil logging. However in OSS, TSL logging is a custom logging library. Abseil logging doesn't recognise the `TF_CPP_MAX_VLOG_LEVEL` and `TF_CPP_VMODULE` environment variables while OSS TSL logging doesn't recognise the `--vmodule` and `--v` flags. To preserve backward compatibility in OSS, we run static initialisation logic that initialises Abseil logging from the `TF_CPP_MIN_LOG_LEVEL`/`TF_CPP_MAX_VLOG_LEVEL`/`TF_CPP_VMODULE` env variables. PiperOrigin-RevId: 799665233
1 parent 20358a1 commit 07a3afe

18 files changed

+278
-1498
lines changed

xla/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,6 @@ xla_cc_test(
264264
"//xla/tsl/platform:statusor",
265265
"//xla/tsl/platform:test_main",
266266
"@com_google_absl//absl/base:log_severity",
267-
"@com_google_absl//absl/log:log_sink",
268267
"@com_google_absl//absl/log:scoped_mock_log",
269268
"@com_google_absl//absl/status",
270269
"@com_google_absl//absl/status:statusor",

xla/service/debug/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,8 @@ xla_cc_test(
6666
deps = [
6767
":unstable_reduction_detector",
6868
"//xla/hlo/parser:hlo_parser",
69-
"//xla/tsl/platform:logging",
7069
"//xla/tsl/platform:statusor",
7170
"@com_google_absl//absl/base:log_severity",
72-
"@com_google_absl//absl/log:log_sink",
7371
"@com_google_absl//absl/log:scoped_mock_log",
7472
"@com_google_absl//absl/status",
7573
"@com_google_absl//absl/status:status_matchers",

xla/service/debug/unstable_reduction_detector_test.cc

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,14 @@ limitations under the License.
1515

1616
#include "xla/service/debug/unstable_reduction_detector.h"
1717

18-
#include <type_traits>
19-
2018
#include <gmock/gmock.h>
2119
#include <gtest/gtest.h>
2220
#include "absl/base/log_severity.h"
23-
#include "absl/log/log_sink.h"
2421
#include "absl/log/scoped_mock_log.h"
2522
#include "absl/status/status.h"
2623
#include "absl/status/status_matchers.h"
2724
#include "absl/strings/string_view.h"
2825
#include "xla/hlo/parser/hlo_parser.h"
29-
#include "xla/tsl/platform/logging.h"
3026
#include "xla/tsl/platform/statusor.h"
3127

3228
namespace xla {
@@ -80,18 +76,16 @@ TEST(UnstableReductionDetectorTest, FailOnUnstableReductions) {
8076
DebugOptions::UNSTABLE_REDUCTION_DETECTION_MODE_FAIL);
8177
UnstableReductionDetector detector;
8278
::absl::ScopedMockLog log;
83-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
84-
EXPECT_CALL(
85-
log,
86-
Log(LogSeverity::kWarning, _,
87-
HasSubstr("1 unstable reductions found in module 'module_main'")));
88-
EXPECT_CALL(log,
89-
Log(LogSeverity::kWarning, _,
90-
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, %init), "
91-
"dimensions={0}, to_apply=%red, "
92-
"metadata={op_name=\"op_name\" "
93-
"source_file=\"source_file.py\" source_line=42}"));
94-
}
79+
EXPECT_CALL(
80+
log,
81+
Log(LogSeverity::kWarning, _,
82+
HasSubstr("1 unstable reductions found in module 'module_main'")));
83+
EXPECT_CALL(log,
84+
Log(LogSeverity::kWarning, _,
85+
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, %init), "
86+
"dimensions={0}, to_apply=%red, "
87+
"metadata={op_name=\"op_name\" "
88+
"source_file=\"source_file.py\" source_line=42}"));
9589
log.StartCapturingLogs();
9690
EXPECT_THAT(
9791
detector.Run(module.get(), /*execution_threads=*/{}),
@@ -110,16 +104,13 @@ TEST(UnstableReductionDetectorTest, WarningOnUnstableReduction) {
110104
DebugOptions::UNSTABLE_REDUCTION_DETECTION_MODE_WARNING);
111105
UnstableReductionDetector detector;
112106
::absl::ScopedMockLog log;
113-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
114-
EXPECT_CALL(log,
115-
Log(LogSeverity::kWarning, _,
116-
"1 unstable reductions found in module 'module_main'"));
117-
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
118-
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, "
119-
"%init), dimensions={0}, to_apply=%red, "
120-
"metadata={op_name=\"op_name\" "
121-
"source_file=\"source_file.py\" source_line=42}"));
122-
}
107+
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
108+
"1 unstable reductions found in module 'module_main'"));
109+
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
110+
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, "
111+
"%init), dimensions={0}, to_apply=%red, "
112+
"metadata={op_name=\"op_name\" "
113+
"source_file=\"source_file.py\" source_line=42}"));
123114
log.StartCapturingLogs();
124115
EXPECT_THAT(detector.Run(module.get(), /*execution_threads=*/{}),
125116
IsOkAndHolds(false));
@@ -135,14 +126,11 @@ TEST(UnstableReductionDetectorTest, FailOnUnstableReductionNoMetadata) {
135126
DebugOptions::UNSTABLE_REDUCTION_DETECTION_MODE_FAIL);
136127
UnstableReductionDetector detector;
137128
::absl::ScopedMockLog log;
138-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
139-
EXPECT_CALL(log,
140-
Log(LogSeverity::kWarning, _,
141-
"1 unstable reductions found in module 'module_main'"));
142-
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
143-
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, "
144-
"%init), dimensions={0}, to_apply=%red"));
145-
}
129+
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
130+
"1 unstable reductions found in module 'module_main'"));
131+
EXPECT_CALL(log, Log(LogSeverity::kWarning, _,
132+
"Unstable reduction: %red.1 = bf16[] reduce(%p0.1, "
133+
"%init), dimensions={0}, to_apply=%red"));
146134
log.StartCapturingLogs();
147135
EXPECT_THAT(detector.Run(module.get(), /*execution_threads=*/{}),
148136
StatusIs(absl::StatusCode::kFailedPrecondition,

xla/service/gpu/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,7 +1762,6 @@ xla_test(
17621762
"@com_google_absl//absl/base:log_severity",
17631763
"@com_google_absl//absl/log",
17641764
"@com_google_absl//absl/log:check",
1765-
"@com_google_absl//absl/log:log_sink",
17661765
"@com_google_absl//absl/log:scoped_mock_log",
17671766
"@com_google_absl//absl/status:status_matchers",
17681767
"@com_google_absl//absl/status:statusor",
@@ -2281,7 +2280,6 @@ xla_test(
22812280
"@com_google_absl//absl/algorithm:container",
22822281
"@com_google_absl//absl/base:log_severity",
22832282
"@com_google_absl//absl/log",
2284-
"@com_google_absl//absl/log:log_sink",
22852283
"@com_google_absl//absl/log:scoped_mock_log",
22862284
"@com_google_absl//absl/status",
22872285
"@com_google_absl//absl/strings:string_view",

xla/service/gpu/gpu_compiler_test.cc

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ limitations under the License.
2525
#include <ostream>
2626
#include <string>
2727
#include <tuple>
28-
#include <type_traits>
2928
#include <utility>
3029
#include <variant>
3130
#include <vector>
@@ -35,7 +34,6 @@ limitations under the License.
3534
#include "absl/base/log_severity.h"
3635
#include "absl/log/check.h"
3736
#include "absl/log/log.h"
38-
#include "absl/log/log_sink.h"
3937
#include "absl/log/scoped_mock_log.h"
4038
#include "absl/status/status_matchers.h"
4139
#include "absl/status/statusor.h"
@@ -1908,15 +1906,12 @@ ENTRY %main {
19081906

19091907
TF_ASSERT_OK_AND_ASSIGN(std::unique_ptr<HloModule> module,
19101908
ParseAndReturnVerifiedModule(kHlo, config));
1911-
// absl::ScopedMockLog only works if we're actually using ABSL logging, and
1912-
// TSL supports a homegrown logging implementation, so we should only check
1913-
// the log is emitted when ABSL logging is used.
1909+
19141910
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
1915-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
1916-
EXPECT_CALL(mock_log,
1917-
Log(absl::LogSeverity::kWarning, EndsWith("/gpu_compiler.cc"),
1918-
StartsWith("Using fallback sort algorithm")));
1919-
}
1911+
EXPECT_CALL(mock_log,
1912+
Log(absl::LogSeverity::kWarning, EndsWith("/gpu_compiler.cc"),
1913+
StartsWith("Using fallback sort algorithm")));
1914+
19201915
// StartCapturingLogs has to be called even if we expect not to capture any
19211916
// logs.
19221917
mock_log.StartCapturingLogs();

xla/service/gpu/gpu_hlo_schedule_test.cc

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,13 @@ limitations under the License.
2222
#include <optional>
2323
#include <string>
2424
#include <tuple>
25-
#include <type_traits>
2625
#include <vector>
2726

2827
#include <gmock/gmock.h>
2928
#include <gtest/gtest.h>
3029
#include "absl/algorithm/container.h"
3130
#include "absl/base/log_severity.h"
3231
#include "absl/log/log.h"
33-
#include "absl/log/log_sink.h"
3432
#include "absl/log/scoped_mock_log.h"
3533
#include "absl/status/status.h"
3634
#include "absl/strings/string_view.h"
@@ -1809,15 +1807,10 @@ TEST_F(GpuHloScheduleTest, LogAnErrorWhenArgumentSizeExceedsMemoryLimit) {
18091807
auto module, ParseAndReturnVerifiedModule(kHloText, module_config));
18101808

18111809
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
1812-
// absl::ScopedMockLog only works if we're actually using ABSL logging, and
1813-
// TSL supports a homegrown logging implementation, so we should only check
1814-
// the log is emitted when ABSL logging is used.
1815-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
1816-
EXPECT_CALL(mock_log,
1817-
Log(absl::LogSeverity::kError, _,
1818-
EndsWith("This indicates an error in the calculation!")))
1819-
.Times(1);
1820-
}
1810+
EXPECT_CALL(mock_log,
1811+
Log(absl::LogSeverity::kError, _,
1812+
EndsWith("This indicates an error in the calculation!")))
1813+
.Times(1);
18211814
mock_log.StartCapturingLogs();
18221815
TF_ASSERT_OK_AND_ASSIGN(auto metadata, ScheduleGpuModule(module.get()));
18231816
EXPECT_EQ(metadata.scheduler_mem_limit, 0);

xla/status_macros_test.cc

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,11 @@ limitations under the License.
1717

1818
#include <functional>
1919
#include <string>
20-
#include <type_traits>
2120
#include <utility>
2221

2322
#include <gmock/gmock.h>
2423
#include <gtest/gtest.h>
2524
#include "absl/base/log_severity.h"
26-
#include "absl/log/log_sink.h"
2725
#include "absl/log/scoped_mock_log.h"
2826
#include "absl/status/status.h"
2927
#include "absl/status/statusor.h"
@@ -93,15 +91,10 @@ TEST(StatusMacros, RetCheckFailingWithExtraMessage) {
9391
}
9492

9593
TEST(StatusMacros, RetCheckLogWarning) {
96-
// absl::ScopedMockLog only works if we're actually using ABSL logging, and
97-
// TSL supports a homegrown logging implementation, so we should only check
98-
// the log is emitted when ABSL logging is used.
9994
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
10095
const std::string kExpectedRegex = "RET_CHECK.*1 == 2 extra message";
101-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
102-
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kWarning, ::testing::_,
103-
::testing::ContainsRegex(kExpectedRegex)));
104-
}
96+
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kWarning, ::testing::_,
97+
::testing::ContainsRegex(kExpectedRegex)));
10598
mock_log.StartCapturingLogs();
10699
absl::Status status =
107100
RetCheckFailWithLogSeverity(absl::LogSeverity::kWarning);
@@ -112,10 +105,8 @@ TEST(StatusMacros, RetCheckLogWarning) {
112105
TEST(StatusMacros, RetCheckLogInfo) {
113106
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
114107
const std::string kExpectedRegex = "RET_CHECK.*1 == 2 extra message";
115-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
116-
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kInfo, ::testing::_,
117-
::testing::ContainsRegex(kExpectedRegex)));
118-
}
108+
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kInfo, ::testing::_,
109+
::testing::ContainsRegex(kExpectedRegex)));
119110
mock_log.StartCapturingLogs();
120111
absl::Status status = RetCheckFailWithLogSeverity(absl::LogSeverity::kInfo);
121112
EXPECT_EQ(status.code(), tsl::error::INTERNAL);
@@ -184,10 +175,8 @@ TEST(StatusMacros, AssignOrReturnUnsuccessfully) {
184175
TEST(StatusMacros, XlaRetCheckFailLogWarning) {
185176
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
186177
const std::string kExpectedLog = "xla ret check fail message";
187-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
188-
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kWarning, ::testing::_,
189-
::testing::HasSubstr(kExpectedLog)));
190-
}
178+
EXPECT_CALL(mock_log, Log(absl::LogSeverity::kWarning, ::testing::_,
179+
::testing::HasSubstr(kExpectedLog)));
191180
mock_log.StartCapturingLogs();
192181
absl::Status status = XlaRetCheckFailLogWarning();
193182
EXPECT_EQ(status.code(), tsl::error::INTERNAL);

xla/tsl/platform/BUILD

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ load(
55
load(
66
"//xla/tsl:tsl.bzl",
77
"if_not_fuchsia",
8+
"if_oss",
89
"internal_visibility",
910
"tsl_copts",
1011
)
1112
load("//xla/tsl:tsl.default.bzl", "get_compatible_with_portable")
1213
load(
1314
"//xla/tsl/platform:build_config.bzl",
14-
"tf_logging_deps",
1515
"tf_platform_alias",
1616
"tf_platform_deps",
1717
"tf_protobuf_compiler_deps",
@@ -51,7 +51,6 @@ exports_files(
5151
"rocm_rocdl_path.h",
5252
"resource_loader.h",
5353
"file_system.cc",
54-
"logging.h",
5554
"file_system.h",
5655
"file_system_helper.cc",
5756
"file_system_helper.h",
@@ -305,6 +304,7 @@ filegroup(
305304
name = "xla_cpu_runtime_srcs",
306305
srcs = [
307306
"dynamic_annotations.h",
307+
"env_time.h",
308308
"macros.h",
309309
],
310310
compatible_with = get_compatible_with_portable(),
@@ -468,12 +468,31 @@ cc_library(
468468

469469
cc_library(
470470
name = "logging",
471+
srcs = ["logging.cc"],
472+
hdrs = ["logging.h"],
471473
compatible_with = get_compatible_with_portable(),
472-
textual_hdrs = ["logging.h"],
473474
visibility = [
474475
"//visibility:public",
475476
],
476-
deps = tf_logging_deps(),
477+
deps = [
478+
"@com_google_absl//absl/log",
479+
"@com_google_absl//absl/log:check",
480+
"@com_google_absl//absl/log:globals",
481+
"@com_google_absl//absl/log:vlog_is_on",
482+
"@com_google_absl//absl/strings:string_view",
483+
] + if_oss([":logging_initializer"]),
484+
)
485+
486+
cc_library(
487+
name = "logging_initializer",
488+
srcs = ["logging_initializer.cc"],
489+
deps = [
490+
"@com_google_absl//absl/base:log_severity",
491+
"@com_google_absl//absl/log:globals",
492+
"@com_google_absl//absl/strings",
493+
"@com_google_absl//absl/strings:string_view",
494+
],
495+
alwayslink = True,
477496
)
478497

479498
tsl_cc_test(
@@ -486,11 +505,11 @@ tsl_cc_test(
486505
":logging",
487506
":statusor",
488507
":test",
489-
"@com_google_absl//absl/base:log_severity",
490508
"@com_google_absl//absl/status",
491509
"@com_google_absl//absl/status:statusor",
492510
"@com_google_absl//absl/strings:str_format",
493511
"@com_google_absl//absl/strings:string_view",
512+
"@com_google_googletest//:gtest",
494513
"@tsl//tsl/platform:path",
495514
"@tsl//tsl/platform:stacktrace_handler",
496515
],
@@ -518,6 +537,9 @@ cc_library(
518537
"@com_google_absl//absl/functional:function_ref",
519538
"@com_google_absl//absl/log",
520539
"@com_google_absl//absl/log:check",
540+
"@com_google_absl//absl/log:log_entry",
541+
"@com_google_absl//absl/log:log_sink",
542+
"@com_google_absl//absl/log:log_sink_registry",
521543
"@com_google_absl//absl/status",
522544
"@com_google_absl//absl/strings",
523545
"@com_google_absl//absl/strings:cord",

xla/tsl/platform/build_config.bzl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ load(
2323
_tf_google_mobile_srcs_only_runtime = "tf_google_mobile_srcs_only_runtime",
2424
_tf_jspb_proto_library = "tf_jspb_proto_library",
2525
_tf_lib_proto_parsing_deps = "tf_lib_proto_parsing_deps",
26-
_tf_logging_deps = "tf_logging_deps",
2726
_tf_platform_alias = "tf_platform_alias",
2827
_tf_platform_deps = "tf_platform_deps",
2928
_tf_portable_deps_no_runtime = "tf_portable_deps_no_runtime",
@@ -62,7 +61,6 @@ tf_google_mobile_srcs_no_runtime = _tf_google_mobile_srcs_no_runtime
6261
tf_google_mobile_srcs_only_runtime = _tf_google_mobile_srcs_only_runtime
6362
tf_jspb_proto_library = _tf_jspb_proto_library
6463
tf_lib_proto_parsing_deps = _tf_lib_proto_parsing_deps
65-
tf_logging_deps = _tf_logging_deps
6664
tf_platform_alias = _tf_platform_alias
6765
tf_platform_deps = _tf_platform_deps
6866
tf_portable_proto_lib = _tf_portable_proto_lib

0 commit comments

Comments
 (0)