[WIP] #96 dotnet core support #204

TheAngryByrd · 2016-11-19T17:18:15Z

This is a work in progress. I've opened this PR early so I can get feedback.

This will add dotnet core support to Logary. Should close #96.

Things that I think need to be done, please correct me if I'm wrong:

TheAngryByrd · 2016-11-19T19:38:17Z

Might have a show stopper for now with StackFrame, which @haf has already commented on: https://github.com/dotnet/corefx/issues/1420#issuecomment-96288400

Looks like it is in net standard 2.0 https://github.com/dotnet/corefx/pull/12527/commits. From my understanding we'll have to wait till then. Unless anyone has a workaround.

haf · 2016-11-20T16:33:50Z

I suggest that we simply remove GetCurrentLogger() from the .Net Core API until .Net standard 2.0 is the expected version so that we don't have to wait. Conditional compile until then.

haf · 2016-11-20T16:34:33Z

src/Logary/DataModel.fs

@@ -1559,7 +1561,7 @@ module Message =

  [<CompiledName "SetTimestamp">]
  let setTimestamp (instant : Instant) msg =
-    { msg with timestamp = instant.Ticks * Constants.NanosPerTick }
+    { msg with timestamp = (instant |> Date.getTicks) * Constants.NanosPerTick }


Prefer the non-piped version when you have only a single argument.

TheAngryByrd · 2016-11-20T16:35:25Z

I do have Logary main compiling with a big hack regarding StackFrame: 25e3607#diff-7ec805bc41571a034ed354f519900efeR35

I'm sure this is WRONG and would cause problems but it does mean that's probably the only place we'll have problems.

haf · 2016-11-20T16:36:11Z

src/Logary/Targets_Core.fs

@@ -597,14 +597,16 @@ module Debugger =
            ack *<= () :> Job<_>

          RingBuffer.take requests ^=> function


All of this target should be conditionally compiled if it doesn't exist on .Net Core. To be honest I never use it and I don't know if someone else it; it's just a 'hey - it's easy to create new targets'-target for me.

haf · 2016-11-20T16:36:53Z

src/Logary/Targets_Core.fs

-      let hourly = FileAge (Duration.FromHours 1L)
-      let daily = FileAge (Duration.FromHours 24L)
-      let weekly = FileAge (Duration.FromHours (7L * 24L))
+      let everyMinute = FileAge (Internals.Date.Duration.fromMinutes 1L)


Could you explain a bit about why you modelled the API like this? E.g. isn't Date implying there's no time: hours, minute, seconds, millis etc?

Just cruft from the Nodatime API changes. Should be back to normal soon.

haf · 2016-11-20T16:37:42Z

src/Logary/Targets_Core.fs

@@ -1224,7 +1228,7 @@ module File =
          if maxSize <= size then true else apply state rest
        | FileAge maxAge :: rest ->
          let created = Instant.FromDateTimeUtc(state.fileInfo.CreationTimeUtc)
-          let age = created - clock.Now
+          let age = created - (clock |> Internals.Date.getInstant)


Single arg, then don't use |>

haf · 2016-11-20T16:38:40Z

src/Logary/Targets_Graphite.fs

@@ -71,7 +71,7 @@ module Impl =
  /// All graphite messages are of the following form.
  /// metric_path value timestamp\n
  let createMsg (path : String) value (timestamp : Instant) =
-    let line = String.Format("{0} {1} {2}\n", path, value, timestamp.Ticks / NodaConstants.TicksPerSecond)
+    let line = String.Format("{0} {1} {2}\n", path, value, (timestamp |> Internals.Date.getTicks ) / NodaConstants.TicksPerSecond)


Perhaps extension method on Instant like instant.ticks? Why shim it in the first place? Can't NodaTime be upgraded instead? https://www.nuget.org/packages/NodaTime/2.0.0-alpha20160729

Yeah I just didn't want to decide to update Nodatime on the full framework. But we totally could do just that and get rid of that nonsense.

It's alright since v4 is still in beta. I haven't frozen the API yet, but we're really getting there. The last couple of weeks have brought tremendous improvements (IMO).

TheAngryByrd · 2016-12-09T22:48:38Z

Since the tests are in expecto, should we bother porting them for now?

haf · 2016-12-10T10:25:41Z

@TheAngryByrd Expecto got .Net Core support yesterday haf/expecto#4

TheAngryByrd · 2016-12-10T16:09:57Z

So I just need to wait for the nuget package right?

haf · 2016-12-12T13:32:02Z

Rebase needed. Expecto has had its PR merged https://github.com/haf/expecto/releases/tag/v2.0.0 and haf/expecto#4 but actually releasing a nuget with it is contingent on this improvement request in albacore: Albacore/albacore#221

Meanwhile expecto could be compiled with .net core support locally to further this PR.

haf · 2016-12-12T13:41:46Z

I'm planning to make DVar a dependency for Logary https://github.com/haf/DVar/releases/tag/v0.1.0 – it's a single file of 125 lines of code that depends on System and System.Threading; so it will need to be ported – or rather – compiled – for .Net core as well.

This will enable the use-case of having targets transparently reconfiguring their endpoints based on load-balancing and services going up or down. Or another use-case could be load-balancing for a cluster (access to three different kafka-servers, rather than a single one), or another one could be client stickyness to a server.

The actual use-case I'm introducing it for is that Windows Performance Counters can suddenly "disappear" which means they have to be re-queried; this happens whenever more than one process with an identical executable-name is started and queries for its own performance counters.

TheAngryByrd · 2016-12-12T22:19:05Z

Ok, I'll try to rebase it tonight.

For expecto, Have you compiled and referenced locally before? The only thing google is telling me is to create a nuget package and host it locally on my file system. Seems like a pain (but i guess this is .net core after all), any guidance is appreciated.

If you're planning on using paket files for the Dvar, it looks like it should just work. Probably just add to the project.json like: https://github.com/logary/logary/blob/102e7b99c3027452ca8ab7653a239813befd3435/src/Logary/project.json#L8

haf-lynx · 2016-12-15T07:52:07Z

Hi Jimmy,

Yes, you can compile it locally to create the package, then place the path of the folder that the nuget is in, inside a paket.local file. Then run paket.restore to override the public nugets with your local ones.

This should work for .Net core as well.

haf · 2016-12-19T20:18:39Z

This is only really blocked on Albacore/albacore#221 now.

haf · 2017-06-28T08:42:25Z

The debugging may not be needed any more? https://github.com/haf/expecto/blob/master/.travis.yml#L28

TheAngryByrd · 2017-06-28T13:33:34Z

I can get it to build on osx locally just fine, I'm just derping over travis for some reason.

TheAngryByrd · 2017-06-30T02:24:17Z

Appveyor: Uhhh... what? Is expecto throwing on whitespace?

Albacore::CommandFailedError: Command failed with status (1):
  src\tests\Logary.Adapters.Facade.Tests\bin\Release\net462\Logary.Adapters.Facade.Tests.exe --sequenced 
ERROR unknown options:

Travis: Whyyyyy, builds locally on osx fine

When using TimeScope
» should contain Running op message (FAIL)
Machine.Specifications.SpecificationException: Should contain "Running op" but is ""
  at Logary.CSharp.Tests.When_using_TimeScope+<>c.<.ctor>b__9_2 () [0x00000] in <8580c6f697cb454fa4a7f95b234c1ac6>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00038] in <dbb16e0bacdc4a0f87478e401bc29b6c>:0 
» should contain Inner message (FAIL)
Machine.Specifications.SpecificationException: Should contain "Inner" but is ""
  at Logary.CSharp.Tests.When_using_TimeScope+<>c.<.ctor>b__9_3 () [0x00000] in <8580c6f697cb454fa4a7f95b234c1ac6>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parame

haf · 2017-06-30T05:58:58Z

@TheAngryByrd My guess is that you've discovered that whitespace can indeed be passed as parameters to processes, just that you need to use the API of the operating system. It sounds like a parsing error in Argu stemming from this, rather than bugs in Expecto.

TheAngryByrd · 2017-06-30T13:31:51Z

Curious as to why it's windows specific though. Guess I'll try to reprodice with Argu.

Thorium · 2017-10-16T10:10:02Z

What is the .NET Coreapp 2.0 status for Logary? This 7/10 seems pretty good, last update at June, and now the Coreapp 2.0 is already released.

Could we get an alpha-Nuget-package, please? :-)

haf reviewed Nov 20, 2016

View reviewed changes

haf mentioned this pull request Dec 19, 2016

.NET Core haf/expecto#1

Closed

Jimmy Byrd added 13 commits June 20, 2017 19:49

Update nodatime for netcore

30c0e5a

Convert src/Logary to new fsproj format

b8cc55f

Convert Logary.Adapters.Facade

8930746

Convert Logary.Tests.fsproj

e224966

Convert Logary.CSharp.csproj

cfb15db

Convert Logary.Adapters.CommonLogging.fsproj

0c22b2b

Convert Logary.Adapters.EventStore.fsproj

6da4de7

Added netcore sln file for easier build of net project style

a80ce59

Convert Logary.Adapters.FsSql.fsproj

69f1c4e

convert Logary.Adapters.log4net.fsproj

4c2c4a3

Convert Logary.Adapters.NLog.fsproj

e7ea5a0

convert Logary.Adapters.Topshelf.fsproj

c1ac751

Convert Logary.CSharp.Facade/Logary.CSharp.Facade.csproj

d4a0334

Jimmy Byrd added 20 commits June 27, 2017 20:27

Convert Logary.Targets.Logstash.Tests.fsproj

689a968

Logary.Targets.Mailgun.Tests.fsproj

5240c3e

Convert Logary.Targets.RabbitMQ.Tests.fsproj

ea948d5

Convert Logary.Targets.Riemann.Tests.fsproj

4951ab1

Convert Logary.Targets.Stackdriver.Tests.fsproj

efa604d

Convert Logary.WinPerfCounters.Tests.fsproj

14c29a5

Convert Logary.Zipkin.Tests.fsproj

a66394a

Add tests to Logary.NetCore.sln

2af93a4

Initial Rakefile change for dotnet core cli

840dd44

Have travis use dotnet core

c6e7715

dotnet core supports Trusty and up

641c7b3

Ensure paket.targets in tools

aba46e7

remove duplicate paket.targets

893824f

Ensure tools/Paket.Restore.targets

c42c027

Set FrameworkPathOverride for travis

5f5337c

Ignore AssemblyInfoVersion for now

1adebbf

Move FrameworkPathOverride to env key in travis

73abd8c

Don't run dotnet clean just yet

5001d1c

travis debugging part1

3c2cc49

travis debugging part2

a804ecf

travis debugging part3

6754e39

Run unit tests in Rakefile

b7cfa47

TheAngryByrd mentioned this pull request Dec 5, 2017

.net core support #96

Closed

haf closed this Feb 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] #96 dotnet core support #204

[WIP] #96 dotnet core support #204

TheAngryByrd commented Nov 19, 2016 •

edited

Loading

TheAngryByrd commented Nov 19, 2016 •

edited

Loading

haf commented Nov 20, 2016

haf Nov 20, 2016

TheAngryByrd commented Nov 20, 2016

haf Nov 20, 2016

haf Nov 20, 2016

TheAngryByrd Nov 20, 2016

haf Nov 20, 2016

haf Nov 20, 2016 •

edited

Loading

TheAngryByrd Nov 20, 2016

haf Nov 20, 2016

TheAngryByrd commented Dec 9, 2016

haf commented Dec 10, 2016

TheAngryByrd commented Dec 10, 2016

haf commented Dec 12, 2016 •

edited

Loading

haf commented Dec 12, 2016 •

edited

Loading

TheAngryByrd commented Dec 12, 2016 •

edited

Loading

haf-lynx commented Dec 15, 2016 •

edited

Loading

haf commented Dec 19, 2016

haf commented Jun 28, 2017

TheAngryByrd commented Jun 28, 2017 •

edited

Loading

TheAngryByrd commented Jun 30, 2017

haf commented Jun 30, 2017

TheAngryByrd commented Jun 30, 2017

Thorium commented Oct 16, 2017

		@@ -597,14 +597,16 @@ module Debugger =
		ack *<= () :> Job<_>

		RingBuffer.take requests ^=> function

[WIP] #96 dotnet core support #204

[WIP] #96 dotnet core support #204

Conversation

TheAngryByrd commented Nov 19, 2016 • edited Loading

TheAngryByrd commented Nov 19, 2016 • edited Loading

haf commented Nov 20, 2016

haf Nov 20, 2016

Choose a reason for hiding this comment

TheAngryByrd commented Nov 20, 2016

haf Nov 20, 2016

Choose a reason for hiding this comment

haf Nov 20, 2016

Choose a reason for hiding this comment

TheAngryByrd Nov 20, 2016

Choose a reason for hiding this comment

haf Nov 20, 2016

Choose a reason for hiding this comment

haf Nov 20, 2016 • edited Loading

Choose a reason for hiding this comment

TheAngryByrd Nov 20, 2016

Choose a reason for hiding this comment

haf Nov 20, 2016

Choose a reason for hiding this comment

TheAngryByrd commented Dec 9, 2016

haf commented Dec 10, 2016

TheAngryByrd commented Dec 10, 2016

haf commented Dec 12, 2016 • edited Loading

haf commented Dec 12, 2016 • edited Loading

TheAngryByrd commented Dec 12, 2016 • edited Loading

haf-lynx commented Dec 15, 2016 • edited Loading

haf commented Dec 19, 2016

haf commented Jun 28, 2017

TheAngryByrd commented Jun 28, 2017 • edited Loading

TheAngryByrd commented Jun 30, 2017

haf commented Jun 30, 2017

TheAngryByrd commented Jun 30, 2017

Thorium commented Oct 16, 2017

TheAngryByrd commented Nov 19, 2016 •

edited

Loading

TheAngryByrd commented Nov 19, 2016 •

edited

Loading

haf Nov 20, 2016 •

edited

Loading

haf commented Dec 12, 2016 •

edited

Loading

haf commented Dec 12, 2016 •

edited

Loading

TheAngryByrd commented Dec 12, 2016 •

edited

Loading

haf-lynx commented Dec 15, 2016 •

edited

Loading

TheAngryByrd commented Jun 28, 2017 •

edited

Loading