Skip to content

Conversation

olt
Copy link
Contributor

@olt olt commented Jul 16, 2018

Allow to use Go's pprof profiler.

This should have no measurable impact when not enabled via TEGOLA_HTTP_PPROF, but I did not found any reliable source for that.

I added a build flag anyway, but can remove it or flip the default (i.e. make it +build pprof).

@olt olt requested review from ARolek and gdey as code owners July 16, 2018 10:54
@olt olt mentioned this pull request Jul 16, 2018
@coveralls
Copy link

coveralls commented Jul 30, 2018

Pull Request Test Coverage Report for Build 1328

  • 1077 of 2151 (50.07%) changed or added relevant lines in 50 files are covered.
  • 26 unchanged lines in 11 files lost coverage.
  • Overall coverage decreased (-3.02%) to 43.97%

Changes Missing Coverage Covered Lines Changed/Added Lines %
maths/clip/intersect/walker.go 1 2 50.0%
maths/makevalid/makevalid.go 14 15 93.33%
maths/hitmap/hitmap.go 0 2 0.0%
server/handle_map_style.go 9 11 81.82%
maths/clip/clip.go 26 28 92.86%
maths/douglaspeucker.go 0 3 0.0%
cmd/internal/register/caches.go 8 11 72.73%
provider/postgis/error.go 0 3 0.0%
dict/types.go 3 6 50.0%
provider/postgis/util.go 29 33 87.88%
Files with Coverage Reduction New Missed Lines %
provider/postgis/util.go 1 64.07%
cache/cache.go 1 37.27%
atlas/atlas.go 1 0.0%
maths/makevalid/plyg/ring.go 1 50.87%
config/config.go 1 56.04%
maths/triangle.go 1 0.3%
maths/points/paired.go 1 0.0%
provider/postgis/postgis.go 2 63.95%
server/middleware_cors.go 2 84.62%
provider/gpkg/binary_header.go 2 100.0%
Totals Coverage Status
Change from base Build 1323: -3.02%
Covered Lines: 5115
Relevant Lines: 11633

💛 - Coveralls

@ARolek
Copy link
Member

ARolek commented Jul 30, 2018

@olt good call on this PR. I think we should flip the build flag to +build pprof and allow the bind string to be set via the env var, i.e. HTTP_TEGOLA_PPROF_BIND=localhost:6060. I think this would be more explicit and flexible.

@olt olt force-pushed the net-http-pprof branch from 76980fa to 1282c5c Compare July 31, 2018 07:16
@olt olt requested a review from JivanAmara as a code owner July 31, 2018 07:16
@olt
Copy link
Contributor Author

olt commented Jul 31, 2018

Changed to +build pprof and made the bind configurable with TEGOLA_HTTP_PPROF_BIND.

I think it's consistent to prefix all envs with TEGOLA_ like TEGOLA_OPTIONS and unlike LAYER_SQL and EXECUTE_SQL.

@ARolek ARolek changed the base branch from master to v0.7.0 July 31, 2018 21:38
@ARolek
Copy link
Member

ARolek commented Jul 31, 2018

@olt that's a good point regarding LAYER_SQL and EXECUTE_SQL. Those are leftover from the early cuts of tegola. I will open an issue to get those streamlined.

@ARolek ARolek merged commit 318002e into go-spatial:v0.7.0 Jul 31, 2018
sacontreras pushed a commit to sacontreras/tegola that referenced this pull request Aug 18, 2018
* 'master' of sacontreras-GitHub:go-spatial/tegola: (125 commits)
  Update README.md
  updating readme and changelog for v0.7.0. go-spatial#490 (go-spatial#492)
  added TEGOLA_ prefix to SQL_DEBUG env var. closes go-spatial#489 (go-spatial#491)
  support for lambda during CI build
  fix for PGX numeric types sharing pointer reference (go-spatial#487)
  make min / max zoom flags consistant. closes go-spatial#480 (go-spatial#483)
  enable Go pprof profiler with TEGOLA_HTTP_PPROF_BIND environment (go-spatial#462)
  Cache seed improvements (go-spatial#454)
  minor documentation polish to postgis layer geometry_type
  provider/postgis: add geometry_type option to avoid table inspection (go-spatial#466)
  Update README.md to correct "Server" heading spelling (go-spatial#476)
  Update README.md (go-spatial#473)
  force remove on CGO builds
  split CGO and non CGO build flow
  updated xgo work flow for cross compile of CGO builds
  set CI_DIR var in build binary CI script
  modified deploy condition in travis
  Issue 420 (go-spatial#471)
  postgis: add support for !bbox! (Mapnik) and !BOX! (MapServer) tokens (go-spatial#461)
  enable Cache-Control headers on S3 caches (go-spatial#448)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants