Skip to content

Commit 993b51b

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: 780164601
1 parent 9ec8192 commit 993b51b

File tree

16 files changed

+256
-1462
lines changed

16 files changed

+256
-1462
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/gpu/BUILD

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,7 +1738,6 @@ xla_test(
17381738
"@com_google_absl//absl/base:log_severity",
17391739
"@com_google_absl//absl/log",
17401740
"@com_google_absl//absl/log:check",
1741-
"@com_google_absl//absl/log:log_sink",
17421741
"@com_google_absl//absl/log:scoped_mock_log",
17431742
"@com_google_absl//absl/status:statusor",
17441743
"@com_google_absl//absl/strings",
@@ -2252,7 +2251,6 @@ xla_test(
22522251
"@com_google_absl//absl/algorithm:container",
22532252
"@com_google_absl//absl/base:log_severity",
22542253
"@com_google_absl//absl/log",
2255-
"@com_google_absl//absl/log:log_sink",
22562254
"@com_google_absl//absl/log:scoped_mock_log",
22572255
"@com_google_absl//absl/status",
22582256
"@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/statusor.h"
4139
#include "absl/strings/str_cat.h"
@@ -1901,15 +1899,12 @@ ENTRY %main {
19011899

19021900
TF_ASSERT_OK_AND_ASSIGN(std::unique_ptr<HloModule> module,
19031901
ParseAndReturnVerifiedModule(kHlo, config));
1904-
// absl::ScopedMockLog only works if we're actually using ABSL logging, and
1905-
// TSL supports a homegrown logging implementation, so we should only check
1906-
// the log is emitted when ABSL logging is used.
1902+
19071903
absl::ScopedMockLog mock_log(absl::MockLogDefault::kIgnoreUnexpected);
1908-
if constexpr (std::is_same_v<absl::LogSink, tsl::TFLogSink>) {
1909-
EXPECT_CALL(mock_log,
1910-
Log(absl::LogSeverity::kWarning, EndsWith("/gpu_compiler.cc"),
1911-
StartsWith("Using fallback sort algorithm")));
1912-
}
1904+
EXPECT_CALL(mock_log,
1905+
Log(absl::LogSeverity::kWarning, EndsWith("/gpu_compiler.cc"),
1906+
StartsWith("Using fallback sort algorithm")));
1907+
19131908
// StartCapturingLogs has to be called even if we expect not to capture any
19141909
// logs.
19151910
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

xla/tsl/platform/default/BUILD

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -285,32 +285,6 @@ cc_library(
285285
]),
286286
)
287287

288-
cc_library(
289-
name = "logging",
290-
srcs = ["logging.cc"],
291-
hdrs = ["//xla/tsl/platform:logging.h"],
292-
tags = [
293-
"manual",
294-
"no_oss",
295-
"nobuilder",
296-
],
297-
textual_hdrs = ["logging.h"],
298-
deps = [
299-
"//xla/tsl/platform:env_time",
300-
"//xla/tsl/platform:macros",
301-
"//xla/tsl/platform:types",
302-
"@com_google_absl//absl/base",
303-
"@com_google_absl//absl/base:core_headers",
304-
"@com_google_absl//absl/base:log_severity",
305-
"@com_google_absl//absl/container:flat_hash_map",
306-
"@com_google_absl//absl/strings",
307-
"@com_google_absl//absl/strings:str_format",
308-
"@com_google_absl//absl/strings:string_view",
309-
"@com_google_absl//absl/synchronization",
310-
"@tsl//tsl/platform",
311-
],
312-
)
313-
314288
filegroup(
315289
name = "xla_cpu_runtime_srcs",
316290
srcs = [
@@ -656,7 +630,6 @@ exports_files(
656630
["*"],
657631
exclude = [
658632
"integral_types.h",
659-
"logging.h",
660633
"test.cc",
661634
],
662635
),
@@ -666,7 +639,6 @@ exports_files(
666639
exports_files(
667640
srcs = [
668641
"integral_types.h",
669-
"logging.h",
670642
"test.cc",
671643
],
672644
visibility = internal_visibility([

xla/tsl/platform/default/build_config.bzl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ def tf_additional_lib_hdrs():
318318
clean_dep("//xla/tsl/platform/default:context.h"),
319319
clean_dep("//xla/tsl/platform/default:criticality.h"),
320320
clean_dep("//xla/tsl/platform/default:integral_types.h"),
321-
clean_dep("//xla/tsl/platform/default:logging.h"),
322321
clean_dep("//xla/tsl/platform/default:stacktrace.h"),
323322
clean_dep("//xla/tsl/platform/default:status.h"),
324323
clean_dep("//xla/tsl/platform/default:statusor.h"),
@@ -549,9 +548,6 @@ def tf_stream_executor_deps(name, platform_dir = "@xla//xla/tsl/platform/"):
549548
def tf_platform_alias(name, platform_dir = "@xla//xla/tsl/platform/"):
550549
return [platform_dir + "default:" + name]
551550

552-
def tf_logging_deps():
553-
return [clean_dep("//xla/tsl/platform/default:logging")]
554-
555551
def tf_error_logging_deps():
556552
return [clean_dep("//xla/tsl/platform/default:error_logging")]
557553

0 commit comments

Comments
 (0)