Report GraphQL stats from alpha.

[arijitAD]

This will help to get stats of graphql+- query adoption.

---

This change is 

[mangalaman93]

Reviewable status: 0 of 5 files reviewed, 10 unresolved discussions (waiting on @arijitAD, @mangalaman93, and @manishrjain)

---

dgraph/cmd/alpha/run.go, line 504 at r1 (raw file):

	if Alpha.Conf.GetBool("telemetry") {
		go edgraph.PeriodicallyPostTelemetry()

should stop this go routine when server shuts down and move this code up before the previous goroutine.

---

dgraph/cmd/zero/zero.go, line 113 at r2 (raw file):

		}
		ms := s.membershipState()
		t := telemetry.NewZeroTelemetry(ms)

Don't have to use telemetry word twice.

---

edgraph/server.go, line 89 at r1 (raw file):

type Server struct{}

// PeriodicallyPostTelemetry periodically report telemetry data for alpha.

minor: reports*

---

edgraph/server.go, line 103 at r2 (raw file):

		}
		ms := worker.GetMembershipState()
		t := telemetry.NewAlphaTelemetry(ms, graphqlQueryCount, nonGraphqlQueryCount)

you can just set the values here itself.

---

edgraph/server.go, line 104 at r2 (raw file):

		ms := worker.GetMembershipState()
		t := telemetry.NewAlphaTelemetry(ms, graphqlQueryCount, nonGraphqlQueryCount)
		if t == nil {

nil check seems unnecessary

---

telemetry/telemetry.go, line 2 at r2 (raw file):

/*
 * Copyright 2018 Dgraph Labs, Inc. and Contributors

2020

---

telemetry/telemetry.go, line 81 at r2 (raw file):

func NewAlphaTelemetry(ms *pb.MembershipState, graphqlQueryCount uint64,
	nonGraphqlQueryCount uint64) *Telemetry {
	t := &Telemetry{

call return here

---

telemetry/telemetry.go, line 111 at r2 (raw file):

		"A0554BAFF14C292A40BC252BB9FF008736A0FD1D44E085")

	client := &http.Client{}

just use the default HTTP client.

---

telemetry/telemetry.go, line 122 at r2 (raw file):

		return err
	}
	glog.V(2).Infof("Telemetry response status: %v", resp.Status)

can log at just one the places

[campoy]

Reviewable status: 0 of 5 files reviewed, 16 unresolved discussions (waiting on @arijitAD, @mangalaman93, and @manishrjain)

---

edgraph/server.go, line 90 at r2 (raw file):

// PeriodicallyPostTelemetry periodically report telemetry data for alpha.
func PeriodicallyPostTelemetry() {

receive a channel or a WaitGroup so you can shutdown this goroutine.

---

edgraph/server.go, line 104 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

nil check seems unnecessary

unnecessary, indeed

---

edgraph/server.go, line 107 at r2 (raw file):

			continue
		}
		t.SinceHours = int(time.Since(start).Hours())

Instead of reporting the uptime, it'd be much more useful if you reported the number of seconds since the last metric was successfully sent (lastPostedAt)

---

edgraph/server.go, line 111 at r2 (raw file):

		err := t.Post()
		glog.V(2).Infof("Telemetry data posted with error: %v", err)

this will print nil every single time this works?

---

telemetry/telemetry.go, line 45 at r2 (raw file):

	Version              string
	GraphqlQueryCount    uint64
	NonGraphqlQueryCount uint64

It'd be better to just count the queries per language

GraphqlQueryCount    uint64
GraphqlpmQueryCount  uint64

---

telemetry/telemetry.go, line 52 at r2 (raw file):

// NewZeroTelemetry returns a Telemetry struct that holds information about the state of zero
// server.
func NewZeroTelemetry(ms *pb.MembershipState) *Telemetry {

This could be called NewZero, since you'll call it telemetry.NewZero

---

telemetry/telemetry.go, line 79 at r2 (raw file):

// NewAlphaTelemetry returns a Telemetry struct that holds information about the state of alpha
// server.
func NewAlphaTelemetry(ms *pb.MembershipState, graphqlQueryCount uint64,

This could be called NewAlpha, since you'll call it telemetry.NewAlpha

[arijitAD]

Reviewable status: 0 of 5 files reviewed, 15 unresolved discussions (waiting on @campoy, @golangcibot, @mangalaman93, and @manishrjain)

---

dgraph/cmd/alpha/run.go, line 504 at r1 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

should stop this go routine when server shuts down and move this code up before the previous goroutine.

Done.

---

dgraph/cmd/zero/zero.go, line 34 at r1 (raw file):

Previously, golangcibot (Bot from GolangCI) wrote…

File is not gofmt-ed with -s (from gofmt)

	"github.com/dgraph-io/dgraph/telemetry"
	"github.com/dgraph-io/dgraph/x"

Done.

---

dgraph/cmd/zero/zero.go, line 113 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

Don't have to use telemetry word twice.

Done.

---

edgraph/server.go, line 90 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

receive a channel or a WaitGroup so you can shutdown this goroutine.

Done.

---

edgraph/server.go, line 103 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

you can just set the values here itself.

Done.

---

edgraph/server.go, line 104 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

unnecessary, indeed

Done.

---

edgraph/server.go, line 107 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

Instead of reporting the uptime, it'd be much more useful if you reported the number of seconds since the last metric was successfully sent (lastPostedAt)

Done.

---

edgraph/server.go, line 111 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

this will print nil every single time this works?

Done.

---

telemetry/telemetry.go, line 2 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

2020

Done.

---

telemetry/telemetry.go, line 45 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

It'd be better to just count the queries per language

GraphqlQueryCount    uint64
GraphqlpmQueryCount  uint64

Done.

---

telemetry/telemetry.go, line 52 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

This could be called NewZero, since you'll call it telemetry.NewZero

Done.

---

telemetry/telemetry.go, line 79 at r2 (raw file):

Previously, campoy (Francesc Campoy) wrote…

This could be called NewAlpha, since you'll call it telemetry.NewAlpha

Done.

---

telemetry/telemetry.go, line 81 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

call return here

Done.

---

telemetry/telemetry.go, line 111 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

just use the default HTTP client.

Done.

---

telemetry/telemetry.go, line 122 at r2 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

can log at just one the places

Done.

[manishrjain]

Reviewed 1 of 2 files at r2, 4 of 4 files at r3.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on @arijitAD, @campoy, @golangcibot, and @mangalaman93)

---

dgraph/cmd/alpha/run.go, line 491 at r3 (raw file):

	go serveHTTP(httpListener, tlsCfg, &wg)

	doneTelemetry := make(chan interface{})

y.Closer is the right thing to use in general.

---

dgraph/cmd/zero/zero.go, line 126 at r3 (raw file):

				lastPostedAt = time.Now()
			} else {
				glog.V(1).Infof("Telemetry couldn't be posted. Error: %v", err)

V(2).

---

edgraph/server.go, line 83 at r3 (raw file):

var (
	graphqlQueryCount   uint64

numQueries
numGraphQL

---

edgraph/server.go, line 113 at r3 (raw file):

			err := t.Post()
			if err == nil {
				atomic.StoreUint64(&graphqlQueryCount, 0)

curVal := atomic.SwapUint64(addr, 0)

curVal should be set in the metrics being sent.

If you fail sending, then I'd do atomic.AddUint64(addr, curVal).

---

edgraph/server.go, line 121 at r3 (raw file):

		case <-doneTelemetry:
			glog.V(2).Infof("Stopping reporting of Telemetry data.")
			wg.Done()

You can just skip stopping the goroutine. No need for closer or WaitGroup.

---

telemetry/telemetry.go, line 2 at r2 (raw file):

Previously, arijitAD (Arijit Das) wrote…

Done.

If this code was moved, then keep the original date.

---

telemetry/telemetry.go, line 49 at r3 (raw file):

}

var keenURL = "https://ping.dgraph.io/3.0/projects/5b809dfac9e77c0001783ad0/events"

var url

[arijitAD]

Reviewable status: 1 of 5 files reviewed, 21 unresolved discussions (waiting on @campoy, @golangcibot, @mangalaman93, and @manishrjain)

---

dgraph/cmd/alpha/run.go, line 491 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

y.Closer is the right thing to use in general.

Done.

---

dgraph/cmd/zero/zero.go, line 126 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

V(2).

Done.

---

edgraph/server.go, line 83 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

numQueries
numGraphQL

Done.

---

edgraph/server.go, line 113 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

curVal := atomic.SwapUint64(addr, 0)

curVal should be set in the metrics being sent.

If you fail sending, then I'd do atomic.AddUint64(addr, curVal).

Done.

---

edgraph/server.go, line 121 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

You can just skip stopping the goroutine. No need for closer or WaitGroup.

Done.

---

telemetry/telemetry.go, line 49 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

var url

Done.