Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix xrootd log level mapping bug and inherit from Pelican log level #1988

Merged
merged 6 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions cmd/config_printer/config_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"io"
"os"
"regexp"
"strings"
"testing"

Expand All @@ -33,22 +34,30 @@ import (
"github.com/pelicanplatform/pelican/config"
)

// Config commands use ansi encoding for color output. This function strips
// those characters for easier comparison of expected and actual output.
func stripAnsiCodes(input string) string {
ansiEscape := regexp.MustCompile(`\x1b\[[0-9;]*m`)
return ansiEscape.ReplaceAllString(input, "")
}

// Mock configuration setup
func setupMockConfig(t *testing.T) error {
// Setting Non-default values, mimics what we'd get from config file
viper.Set("Logging.Level", "trace")
viper.Set("Logging.Cache.Http", "info")
viper.Set("Logging.Cache.Xrootd", "info")
viper.Set("Logging.Origin.Http", "info")

// Set default config
config.SetBaseDefaultsInConfig(viper.GetViper())
viper.Set("ConfigDir", t.TempDir())
config.InitConfig()
if err := config.SetServerDefaults(viper.GetViper()); err != nil {
return err
}
if err := config.SetClientDefaults(viper.GetViper()); err != nil {
return err
}
// Setting Non-default values
viper.Set("Logging.Cache.Http", "info")
viper.Set("Logging.Cache.Xrootd", "info")
viper.Set("Logging.Level", "info")
viper.Set("Logging.Origin.Http", "info")

return nil
}
Expand Down Expand Up @@ -76,21 +85,21 @@ func TestConfigGet(t *testing.T) {
{
name: "no-arguments",
args: []string{},
expected: []string{`logging.cache.http: "info"`, `logging.cache.xrootd: "info"`, `logging.level: "info"`, `logging.origin.http: "info"`},
expected: []string{`logging.cache.http: "info"`, `logging.cache.xrootd: "info"`, `logging.level: "trace"`, `logging.origin.http: "info"`},
notExpected: []string{},
},
{
name: "match-http",
args: []string{"Http", "level"},
expected: []string{`logging.cache.http: "info"`, `logging.level: "info"`, `logging.origin.http: "info"`},
expected: []string{`logging.cache.http: "info"`, `logging.level: "trace"`, `logging.origin.http: "info"`},
notExpected: []string{`logging.cache.xrootd: "info"`},
},

{
name: "match-http-with-origin-flag",
args: []string{"Http", "-m", "origin"},
expected: []string{`logging.origin.http: "info"`},
notExpected: []string{`logging.cache.http: "info"`, `logging.cache.xrootd: "info"`, `logging.level: "info"`},
notExpected: []string{`logging.cache.http: "info"`, `logging.cache.xrootd: "info"`, `logging.level: "trace"`},
},
}

Expand Down Expand Up @@ -119,7 +128,7 @@ func TestConfigGet(t *testing.T) {
<-done
os.Stdout = oldStdout

got := strings.TrimSpace(strings.ToLower(buf.String()))
got := strings.TrimSpace(strings.ToLower(stripAnsiCodes(buf.String())))

for _, expected := range expectedLines {
expectedLower := strings.ToLower(expected)
Expand Down Expand Up @@ -185,9 +194,9 @@ func TestConfigSummary(t *testing.T) {
<-done
os.Stdout = oldStdout

got := strings.TrimSpace(strings.ToLower(buf.String()))
got := strings.TrimSpace(strings.ToLower(stripAnsiCodes(buf.String())))

expectedLines := []string{`logging:`, ` cache:`, ` origin:`, ` level: info`, ` http: info`}
expectedLines := []string{`logging:`, ` cache:`, ` origin:`, ` level: trace`, ` http: info`}
notExpectedLines := []string{`debug: true`}

for _, expected := range expectedLines {
Expand Down
39 changes: 39 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,45 @@ func SetServerDefaults(v *viper.Viper) error {
// we want to be able to check if this is user-provided (which we can't do for defaults.yaml)
v.SetDefault(param.Origin_S3UrlStyle.GetName(), "path")

// At the time of this comment, Pelican's default log level is set to "Error". However, that's still
// too verbose for some XRootD parameters. Because we generally want to use Pelican's configured log level as a
// default for the XRootD parameters, we only set the corrected default values for these special XRootD directives
// when Pelican is running in its own default Error level. Otherwise we use Pelican's configured log level as a
// default for other params.
defaultLevel := log.GetLevel().String()
for _, param := range []param.StringParam{
param.Logging_Origin_Cms,
param.Logging_Origin_Xrd,
param.Logging_Origin_Ofs,
param.Logging_Origin_Oss,
param.Logging_Origin_Http,
param.Logging_Cache_Ofs,
param.Logging_Cache_Pss,
param.Logging_Cache_PssSetOpt,
param.Logging_Cache_Http,
param.Logging_Cache_Xrd,
param.Logging_Cache_Xrootd,
} {
v.SetDefault(param.GetName(), defaultLevel)
}

// If Pelican is at its default error level, do our custom mapping
if defaultLevel == log.ErrorLevel.String() {
v.SetDefault(param.Logging_Origin_Scitokens.GetName(), "fatal")
v.SetDefault(param.Logging_Origin_Xrootd.GetName(), "info")
v.SetDefault(param.Logging_Cache_Scitokens.GetName(), "fatal")
v.SetDefault(param.Logging_Cache_Pfc.GetName(), "info")
} else { // Otherwise treat them as any other log level
for _, param := range []param.StringParam{
param.Logging_Origin_Scitokens,
param.Logging_Origin_Xrootd,
param.Logging_Cache_Scitokens,
param.Logging_Cache_Pfc,
} {
v.SetDefault(param.GetName(), defaultLevel)
}
}

if IsRootExecution() {
v.SetDefault(param.Origin_RunLocation.GetName(), filepath.Join("/run", "pelican", "xrootd", "origin"))
v.SetDefault(param.Cache_RunLocation.GetName(), filepath.Join("/run", "pelican", "xrootd", "cache"))
Expand Down
18 changes: 1 addition & 17 deletions config/resources/defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,7 @@
#

Logging:
Level: "Error"
Origin:
Cms: error
Http: error
Ofs: error
Oss: error
Scitokens: fatal
Xrd: error
Xrootd: info
Cache:
Http: error
Ofs: error
Pfc: info
Pss: error
Scitokens: fatal
Xrd: error
Xrootd: error
Level: error
Client:
SlowTransferRampupTime: 100s
SlowTransferWindow: 30s
Expand Down
63 changes: 60 additions & 3 deletions docs/parameters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,10 @@ components: ["director"]
name: Logging.Level
description: |+
A string defining the log level of the client. Options include (going from most info to least): Trace, Debug, Info, Warn, Error, Fatal, Panic.

Log levels are inherited by all components unless explicitly overridden. Levels are case-insensitive.
type: string
default: Error
default: error
components: ["*"]
---
name: Logging.LogLocation
Expand All @@ -199,6 +201,9 @@ description: |+
connect to a master one informing it if a server is available. XRootD asks cms where a file
could be found and cms works to report back the server for where the file is located.
Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["origin"]
Expand All @@ -207,6 +212,9 @@ name: Logging.Origin.Scitokens
description: |+
Trace level of scitokens debug output within XRootD configuration. This entails token management
and security credentials within XRootD. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: fatal
components: ["origin"]
Expand All @@ -215,6 +223,9 @@ name: Logging.Origin.Xrd
description: |+
Trace level of the eXtended Request Daemon within XRootD, another main XRootD executable. This reports information
the XRootD protocol and works with cms. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["origin"]
Expand All @@ -223,7 +234,11 @@ name: Logging.Origin.Xrootd
description: |+
Trace options for XRootD debug output within XRootD configuration. This prefix is reserved for the xroot protocol,
which is the component that sits on sockets and talks to clients as they query file-system info, open files, and read data.
This is the protocol for XRootD (like http) and handles connections and requests. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`
This is the protocol for XRootD (like http) and handles connections and requests. Accepted values: `trace`, `debug`, `info`,
`warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: info
components: ["origin"]
Expand All @@ -233,6 +248,9 @@ description: |+
Logging level for the HTTP component of the origin. Increasing to debug
will cause the Xrootd daemon to log all headers and requests.
Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["origin"]
Expand All @@ -241,6 +259,9 @@ name: Logging.Origin.Ofs
description: |+
Logging level of Xrootd's "Open File System" (ofs) subsystem. The OFS manages the file descriptor table and redirection/
error handling. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["origin"]
Expand All @@ -249,6 +270,9 @@ name: Logging.Origin.Oss
description: |+
Logging level of Xrootd's "Open Storage System" (oss) subsystem. The OSS manages the interaction with the underlying
POSIX storage (open, read, write, close, etc). Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["origin"]
Expand All @@ -257,6 +281,9 @@ name: Logging.Cache.Http
description: |+
Logging level for the HTTP component of the cache. Increasing to debug will cause the Xrootd daemon to log
all headers and requests. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
Expand All @@ -266,6 +293,9 @@ description: |+
Trace level of XRootD's Open File System. This component cares about files and directories from the administrative perspective.
This component is build on top of the Open Storage System component, which deals with things like file creation and reads and
writes for files and directories. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
Expand All @@ -275,14 +305,32 @@ description: |+
Trace level of XRootD Proxy File Cache (XCache), the caching mechanism used by XRootD. This component
entails information for caches/caching within XRootD. This component instantiates its own Open Storage
System (OSS) to write local files to. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: info
components: ["cache"]
---
name: Logging.Cache.Pss
description: |+
Trace level of XRootD Proxy System Service. Variables this component reports include: number of remotes file opens,
number of opens that failed, number of remote file closes, and number of closes that failed. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`
number of opens that failed, number of remote file closes, and number of closes that failed. Accepted values: `trace`, `debug`,
`info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
---
name: Logging.Cache.PssSetOpt
description: |+
Trace level of XRootD Proxy System Service Set Options. This component reports detailed information about the configuration
and operational settings of the Proxy System Service. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
Expand All @@ -291,6 +339,9 @@ name: Logging.Cache.Scitokens
description: |+
Trace level of scitokens debug output within XRootD configuration. This entails token management
and security credentials within XRootD. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: fatal
components: ["cache"]
Expand All @@ -299,6 +350,9 @@ name: Logging.Cache.Xrd
description: |+
Trace level of the eXtended Request Daemon within XRootD, another main XRootD executable. This reports information
the XRootD protocol and works with cms. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
Expand All @@ -308,6 +362,9 @@ description: |+
Trace options for XRootD debug output within XRootD configuration. This prefix is reserved for the xroot protocol,
which is the component that sits on sockets and talks to clients as they query file-system info, open files, and read data.
This is the protocol for XRootD (like http) and handles connections and requests. Accepted values: `trace`, `debug`, `info`, `warn`, `error`, `fatal`, `panic`

If a non-default value is configured for `Logging.Level`, that level will be inherited unless explicitly
overridden here. Levels are case-insensitive.
type: string
default: error
components: ["cache"]
Expand Down
1 change: 1 addition & 0 deletions param/parameters.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions param/parameters_struct.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading