Skip to content

Commit

Permalink
refactor(GraphQL): Rename logRequest name and its resolver. (#8388)
Browse files Browse the repository at this point in the history
## Problem

In order to add GraphQL Query Log(#8305) we need to rename the current
DQL query log resolver and config.

## Solution

> Rename: logRequest => logDQLRequest 

Important change:

```diff
--- glog.Infof("Got a query: %+v", req.req)
+++ glog.Infof("Got a DQL query: %+v", req.req)
```
  • Loading branch information
MichelDiz authored Nov 12, 2022
1 parent f729963 commit bc5f584
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 26 deletions.
4 changes: 2 additions & 2 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1188,8 +1188,8 @@ func (s *Server) doQuery(ctx context.Context, req *Request) (resp *api.Response,
return nil, serverOverloadErr
}

if bool(glog.V(3)) || worker.LogRequestEnabled() {
glog.Infof("Got a query: %+v", req.req)
if bool(glog.V(3)) || worker.LogDQLRequestEnabled() {
glog.Infof("Got a DQL query: %+v", req.req)

This comment has been minimized.

Copy link
@damonfeldman

damonfeldman Nov 29, 2022

Because this shows in the logs, it sounds like the server "got a DQL query" but in fact the server may have received a GraphQL query who's DQL form is in req.req.

I suggest rewording this to "Got a query. DQL form: %+v" to clarify that we are viewing the DQL form, but any query will be logged here including GraphQL.

This comment has been minimized.

Copy link
@MichelDiz

MichelDiz Nov 29, 2022

Author Contributor

It gets a GraphQL query as colateral. But this execution is DQL indeed. All GraphQL queries are converted into DQL. I have a Log for GraphQL here #8428

}

isGraphQL, _ := ctx.Value(IsGraphql).(bool)
Expand Down
6 changes: 3 additions & 3 deletions graphql/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,10 @@ const (
cacheMb: Float
"""
True value of logRequest enables logging of all the requests coming to alphas.
False value of logRequest disables above.
True value of logDQLRequest enables logging of all the requests coming to alphas.
False value of logDQLRequest disables above.
"""
logRequest: Boolean
logDQLRequest: Boolean
}
type ConfigPayload {
Expand Down
14 changes: 7 additions & 7 deletions graphql/admin/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ import (

type configInput struct {
CacheMb *float64
// LogRequest is used to update WorkerOptions.LogRequest. true value of LogRequest enables
// logging of all requests coming to alphas. LogRequest type has been kept as *bool instead of
// bool to avoid updating WorkerOptions.LogRequest when it has default value of false.
LogRequest *bool
// LogDQLRequest is used to update WorkerOptions.LogDQLRequest. true value of LogDQLRequest enables
// logging of all requests coming to alphas. LogDQLRequest type has been kept as *bool instead of
// bool to avoid updating WorkerOptions.LogDQLRequest when it has default value of false.
LogDQLRequest *bool
}

func resolveUpdateConfig(ctx context.Context, m schema.Mutation) (*resolve.Resolved, bool) {
Expand All @@ -50,9 +50,9 @@ func resolveUpdateConfig(ctx context.Context, m schema.Mutation) (*resolve.Resol
}
}

// input.LogRequest will be nil, when it is not specified explicitly in config request.
if input.LogRequest != nil {
worker.UpdateLogRequest(*input.LogRequest)
// input.LogDQLRequest will be nil, when it is not specified explicitly in config request.
if input.LogDQLRequest != nil {
worker.UpdateLogDQLRequest(*input.LogDQLRequest)
}

return resolve.DataResult(
Expand Down
6 changes: 3 additions & 3 deletions wiki/content/graphql/admin/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ Here are the important types, queries, and mutations from the `admin` schema.
cacheMb: Float

"""
True value of logRequest enables logging of all the requests coming to alphas.
False value of logRequest disables above.
True value of logDQLRequest enables logging of all the requests coming to alphas.
False value of logDQLRequest disables above.
"""
logRequest: Boolean
logDQLRequest: Boolean
}

type ConfigPayload {
Expand Down
14 changes: 7 additions & 7 deletions worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,17 @@ func UpdateCacheMb(memoryMB int64) error {
return nil
}

// UpdateLogRequest updates value of x.WorkerConfig.LogRequest.
func UpdateLogRequest(val bool) {
// UpdateLogDQLRequest updates value of x.WorkerConfig.LogDQLRequest.
func UpdateLogDQLRequest(val bool) {
if val {
atomic.StoreInt32(&x.WorkerConfig.LogRequest, 1)
atomic.StoreInt32(&x.WorkerConfig.LogDQLRequest, 1)
return
}

atomic.StoreInt32(&x.WorkerConfig.LogRequest, 0)
atomic.StoreInt32(&x.WorkerConfig.LogDQLRequest, 0)
}

// LogRequestEnabled returns true if logging of requests is enabled otherwise false.
func LogRequestEnabled() bool {
return atomic.LoadInt32(&x.WorkerConfig.LogRequest) > 0
// LogDQLRequestEnabled returns true if logging of requests is enabled otherwise false.
func LogDQLRequestEnabled() bool {
return atomic.LoadInt32(&x.WorkerConfig.LogDQLRequest) > 0
}
8 changes: 4 additions & 4 deletions x/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,11 @@ type WorkerOptions struct {
Security *z.SuperFlag
// EncryptionKey is the key used for encryption at rest, backups, exports. Enterprise only feature.
EncryptionKey SensitiveByteSlice
// LogRequest indicates whether alpha should log all query/mutation requests coming to it.
// Ideally LogRequest should be a bool value. But we are reading it using atomics across
// queries hence it has been kept as int32. LogRequest value 1 enables logging of requests
// LogDQLRequest indicates whether alpha should log all query/mutation requests coming to it.
// Ideally LogDQLRequest should be a bool value. But we are reading it using atomics across
// queries hence it has been kept as int32. LogDQLRequest value 1 enables logging of requests
// coming to alphas and 0 disables it.
LogRequest int32
LogDQLRequest int32
// If true, we should call msync or fsync after every write to survive hard reboots.
HardSync bool
// Audit contains the audit flags that enables the audit.
Expand Down

0 comments on commit bc5f584

Please sign in to comment.