The Redirector



  • We have several processes that send messages to each other: A sends to B which sends to C. There are configuration files for queues between each process. A and B should point to a common broker/queue, and B and C should point to a common broker/queue. Prior to an upcoming release, I forced the various teams to do a system-wide performance test. They complained that they didn't have the time and there was no need. I insisted.

    One business case was that a user of A sent a message to B which did minor enrichment and forwarded it to C, which was to do real work, and send a response back to B which would send a response back to A. The business user reported that the response was very slow. The developers on B stated that things were flying, so it must be C's fault. Our team was monitoring C and saw no messages coming in.

    Wait; how can A be seeing any results if nothing is ever being requested of C? Ah ha! something must be pointing to another instance somewhere else. Easy solution; just check all the configurations.

    Unfortunately, everything looked good, yet A was getting slow responses, B was passing messages through and C was never receiving them. On a hunch, I poked into B's code and found the following:

      private String ipRedirect(String sourceIP) {
    if ("1.2.3.4".equals(sourceIP)) return sourceIP;
    if ("1.2.3.5".equals(sourceIP)) return sourceIP;
    // ...
    if ("1.2.3.8".equals(sourceIP)) return "1.2.3.44";
    // ...
    return sourceIP;
    }

    That's right, some genius put in a little configuration-bypass to redirect stuff back at his own laptop so he could run a local instance of C, and never took it out of the code.

    If we hadn't run this test, it would've gotten released to production with everything hanging off of a local copy of unreleased software running on a roaming laptop.

    I can just imagine the production problem reports as the guy took his laptop and air-card into and out of dead zones: While the C servers are sitting there, unaccessed, users of A are complaining that the system (B/C) is not responding; Wait it is; Nope; Yep; Nope; Yep; WTF?!

     



  • @snoofle said:

    That's right, some genius put in a little configuration-bypass to redirect stuff back at his own laptop so he could run a local instance of C, and never took it out of the code.

    If we hadn't run this test, it would've gotten released to production with everything hanging off of a local copy of unreleased software running on a roaming laptop.

    I can just imagine the production problem reports as the guy took his laptop and air-card into and out of dead zones: While the C servers are sitting there, unaccessed, users of A are complaining that the system (B/C) is not responding; Wait it is; Nope; Yep; Nope; Yep; WTF?!

     

    Have you identified said genius?

    If so, has this person been hung up by their thumbs?



  • @drobnox said:

    Have you identified said genius?
    Source control gave him up. His name is being discussed by the higher-ups as I write this.

    I don't mind folks making temporary changes to get around other teams' development systems that might not be running; but this code was committed and left in place for 3 months (since just after the freeze for the prior release). If this had made it to production, our customers would have gone berserk, and rightfully so!



  • @snoofle said:

    If we hadn't run this test, it would've gotten released to production with everything hanging off of a local copy of unreleased software running on a roaming laptop.

    really? The production is on the same network as the guy's development laptop?!

    Surely not.



  •  Can I have the hotdog franchise at the execution?



  • @pure said:

    @snoofle said:
    If we hadn't run this test, it would've gotten released to production with everything hanging off of a local copy of unreleased software running on a roaming laptop.

    really? The production is on the same network as the guy's development laptop?!

     

    Surely not.

    It's a flat network - everything is visible from everywhere else. It's supposed to be kept separate via restricted logins, but I've posted plenty of times how everyone has access...


  • @snoofle said:

    @drobnox said:

    Have you identified said genius?
    Source control gave him up. His name is being discussed by the higher-ups as I write this.

    I don't mind folks making temporary changes to get around other teams' development systems that might not be running; but this code was committed and left in place for 3 months (since just after the freeze for the prior release). If this had made it to production, our customers would have gone berserk, and rightfully so!

    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?



  • @JoeCool said:

    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?

    Indeed. Stupid mistakes happen now and than and always will, since humans are just humans. That's why it's important to have all the tests and reviews and such in place and why this occurrence should primarily be used to sell up to higher ups that system-wide performance test is useful and they too should always insist on doing it. And perhaps also that it would be useful to finally put firewall between the network running prod and preprod and the network running devel and test so that anything staged to preprod can't, even accidentally, depend on anything not there yet (along with any other benefits like better security).



  • @Bulb said:

    @JoeCool said:
    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?

    Indeed. Stupid mistakes happen now and than and always will, since humans are just humans. That's why it's important to have all the tests and reviews and such in place and why this occurrence should primarily be used to sell up to higher ups that system-wide performance test is useful and they too should always insist on doing it. And perhaps also that it would be useful to finally put firewall between the network running prod and preprod and the network running devel and test so that anything staged to preprod can't, even accidentally, depend on anything not there yet (along with any other benefits like better security).

    I agree... its easy to check in some debug code, when you're tired, or whatever.
    And it is for this reason that you have a proper source control system, with revision history etc.
    This is also why you have a number of business processes like test deployments, etc, to make sure everything is ok before it gets shipped. The real WTF is that someone wanted to skip these processes.



  • Who are you and what have you done with the real Snoofle?

    So, to summarise: some debug code was accidentally checked-in to source control, it was found in testing before release and fixed.

    everything went better than expected

    This is not how Snoofle's posts normally go! 



  • @JoeCool said:

    @snoofle said:

    @drobnox said:

    Have you identified said genius?
    Source control gave him up. His name is being discussed by the higher-ups as I write this.

    I don't mind folks making temporary changes to get around other teams' development systems that might not be running; but this code was committed and left in place for 3 months (since just after the freeze for the prior release). If this had made it to production, our customers would have gone berserk, and rightfully so!

    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?

    Did you miss the part where snoofle said he had to force the devs to do testing? He works at WTF Inc. where process is nonexistent. Lack of process == mistakes, but also means management have a scapegoat when things go wrong. Typical dysfunctional corporate mindset.



  • @JoeCool said:

    @snoofle said:

    @drobnox said:

    Have you identified said genius?
    Source control gave him up. His name is being discussed by the higher-ups as I write this.

    I don't mind folks making temporary changes to get around other teams' development systems that might not be running; but this code was committed and left in place for 3 months (since just after the freeze for the prior release). If this had made it to production, our customers would have gone berserk, and rightfully so!

    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?

     

     There is a difference between writing:

    Send.redirect("My IP");

    And:

    //TODO: Remove this stupid testing code before release

    @GenereteSomeWarning

    Send.redirect("My IP");

     



  •  Correct values for the log file: B.canConnect = { true, false, Laptop_Not_Found }.



  • @The_Assimilator said:

    Did you miss the part where snoofle said he had to force the devs to do testing? He works at WTF Inc. where process is nonexistent. Lack of process == mistakes, but also means management have a scapegoat when things go wrong. Typical dysfunctional corporate mindset.

    No, because you invented that part. At no point did he say he forced the devs to do testing. He said he had to force various teams to do performance testing.
    And obviously, this guy DID do testing, otherwise he wouldn't have test code to accidentally commit.
    The WTF is that this guy is the scapegoat, and that apparently snoofle thinks that is the right thing.



  • @fire2k said:

    @JoeCool said:

    @snoofle said:

    @drobnox said:

    Have you identified said genius?
    Source control gave him up. His name is being discussed by the higher-ups as I write this.

    I don't mind folks making temporary changes to get around other teams' development systems that might not be running; but this code was committed and left in place for 3 months (since just after the freeze for the prior release). If this had made it to production, our customers would have gone berserk, and rightfully so!

    So the guy accidentally checked it in. Not sure why he needs to be hung up by his thumbs, or the higher-ups discuss his name. Why didn't the regular QA cycle catch this? Why didn't the code review process catch this?

     

     There is a difference between writing:

    Send.redirect("My IP");

    And:

    //TODO: Remove this stupid testing code before release

    @GenereteSomeWarning

    Send.redirect("My IP");

     

    Yes, in one case you have no warning, and in the other case, you get a warning buried somewhere in the other 10,000+ warnings.



  • @JoeCool said:

    @fire2k said:

    There is a difference between writing:

    Send.redirect("My IP");

    And:

    //TODO: Remove this stupid testing code before release

    @GenereteSomeWarning

    Send.redirect("My IP");

     

    Yes, in one case you have no warning, and in the other case, you get a warning buried somewhere in the other 10,000+ warnings.

    The most important coding standard rule of all: Code must at all times compile without any warnings.




  • @fire2k said:

    There is a difference between writing:
    Send.redirect("My IP");
    And:
    //TODO: Remove this stupid testing code before release
    @GenerateSomeWarning
    Send.redirect("My IP");

    There is, but they're both the wrong way to do it. B should be getting C's address from a config file, and ideally there should be separate revision-controlled config files for dev, staging, and prod.



  • @DaveK said:

    @JoeCool said:
    @fire2k said:

    There is a difference between writing:

    Send.redirect("My IP");

    And:

    //TODO: Remove this stupid testing code before release

    @GenereteSomeWarning

    Send.redirect("My IP");

     

    Yes, in one case you have no warning, and in the other case, you get a warning buried somewhere in the other 10,000+ warnings.

    The most important coding standard rule of all: Code must at all times compile without any warnings.


    Hahahahaha!
    I wish reality were this way.



  • @JoeCool said:

    @DaveK said:
    @JoeCool said:
    @fire2k said:

    There is a difference between writing:

    Send.redirect("My IP");

    And:

    //TODO: Remove this stupid testing code before release

    @GenereteSomeWarning

    Send.redirect("My IP");

     

    Yes, in one case you have no warning, and in the other case, you get a warning buried somewhere in the other 10,000+ warnings.

    The most important coding standard rule of all: Code must at all times compile without any warnings.

    Hahahahaha!
    I wish reality were this way.

    Everywhere I've worked where it wasn't already the case, I have made damn sure it became so.




  • @pjt33 said:

    @fire2k said:
    There is a difference between writing:

    Send.redirect("My IP");
    And:
    //TODO: Remove this stupid testing code before release
    @GenerateSomeWarning
    Send.redirect("My IP");

    There is, but they're both the wrong way to do it. B should be getting C's address from a config file, and ideally there should be separate revision-controlled config files for dev, staging, and prod.
     

     What if I don't want the IP to be easily changeable through a config file? Maybe it's a deployed app and we don't want the in-house guys randomly redirecting stuff?

     



  • @JoeCool said:

    @DaveK said:
    The most important coding standard rule of all: Code must at all times compile without any warnings.


    Hahahahaha!
    I wish reality were this way.

    Oh, don't even get me started. Imagine something like this little bit of pseudocode implemented in a language where the result is, by default, only a warning, not an error, given A_WIDTH != B_WIDTH:

    class A 
    {
        private someType[A_WIDTH] a;
        public someType[A_WIDTH] getA() { return a; }
    }
    
    class B 
    {
        private someType[B_WIDTH] b;
        public  setB(someType[B_WIDTH] b) { this.b = b; }
    }
    
    A a = new A();
    B b = new B();
    
    // Initialize a with something 
    // ...
    
    b.setB(a.getA); 
    

    Hundreds of such warnings are routinely ignored, for years. "It's only a warning. There are no errors, and all the tests pass, so it must be good. Release it."



  • @fire2k said:

    What if I don't want the IP to be easily changeable through a config file? Maybe it's a deployed app and we don't want the in-house guys randomly redirecting stuff?

    That seems like really bad practice. If you have to hard-code IPs just to get around your IT people, there's something very wrong. Meanwhile, hard-coding is awful because what if somebody needs to change the value?



  • @HardwareGeek said:

    Hundreds of such warnings are routinely ignored, for years. "It's only a warning. There are no errors, and all the tests pass, so it must be good. Release it."

    I never said I thought it was OK, so no need for a fucking sample. The reality is, you come into a job, the product has thousands of warnings, you stress that they should be fixed. People agree, it doesn't change. It is always: we will do it later.



  • @morbiuswilters said:

    @fire2k said:
    What if I don't want the IP to be easily changeable through a config file? Maybe it's a deployed app and we don't want the in-house guys randomly redirecting stuff?

    That seems like really bad practice. If you have to hard-code IPs just to get around your IT people, there's something very wrong. Meanwhile, hard-coding is awful because what if somebody needs to change the value?

     

    If proximity to both my cousin and a class of computer science students has taught me anything it's that in case of error, some people will change random shit in config files. Yes, I need to recompile after hardcoding, but if it's in-house who cares? I'd agree with you on principle though.



  • @HardwareGeek said:

    Imagine something like this little bit of pseudocode implemented in a language where the result is, by default, only a warning, not an error, given A_WIDTH != B_WIDTH:

    What language actually lets you set the size of an array return type? That would be handy!



  • @ekolis said:

    @HardwareGeek said:
    Imagine something like this little bit of pseudocode implemented in a language where the result is, by default, only a warning, not an error, given A_WIDTH != B_WIDTH:

    What language actually lets you set the size of an array return type? That would be handy!

    Go



  • @ekolis said:

    @HardwareGeek said:
    Imagine something like this little bit of pseudocode implemented in a language where the result is, by default, only a warning, not an error, given A_WIDTH != B_WIDTH:

    What language actually lets you set the size of an array return type? That would be handy!

    It's really port connections in a hardware description language. I figured it was easier to write a little OO pseudocode, the meaning of which would be obvious to everyone here, than explain the hardware description language syntax.

    Verilog does allow functions to return multi-bit values, which must be of a specified number of bits, but that's not quite the same thing as an array of bits, which has a subtly different syntax and considerably different semantics.



  • @HardwareGeek said:

    @ekolis said:
    @HardwareGeek said:
    Imagine something like this little bit of pseudocode implemented in a language where the result is, by default, only a warning, not an error, given A_WIDTH != B_WIDTH:

    What language actually lets you set the size of an array return type? That would be handy!

    It's really port connections in a hardware description language. I figured it was easier to write a little OO pseudocode, the meaning of which would be obvious to everyone here, than explain the hardware description language syntax.

    Verilog does allow functions to return multi-bit values, which must be of a specified number of bits, but that's not quite the same thing as an array of bits, which has a subtly different syntax and considerably different semantics.

    He doesn't care. He just likes to talk about Go.



  • @morbiuswilters said:

    He doesn't care. He just likes to talk about Go.

    Please, no, anything but that!

    Are you saying ekolis wants to talk about Go? I thought only Ben L. was that demented.



  • @HardwareGeek said:

    @morbiuswilters said:
    He doesn't care. He just likes to talk about Go.

    Please, no, anything but that!

    Are you saying ekolis wants to talk about Go? I thought only Ben L. was that demented.

    In my defense...

    cheese it!!



  • @morbiuswilters said:

    In my defense...

    cheese it!!

    OMG Cheez Its



    the best part of that video if the $100 trick.



  • @Ronald said:

    @morbiuswilters said:
    In my defense...

    cheese it!!

    OMG Cheez Its



    the best part of that video if the $100 trick.

    Not bad.



  • @fire2k said:

    If proximity to both my cousin and a class of computer science students has taught me anything it's that in case of error, some people will change random shit in config files. Yes, I need to recompile after hardcoding, but if it's in-house who cares?

    I think that I would prefer to address that concern by encrypting the config file than by (effectively) embedding it inside the executable.


  • ♿ (Parody)

    @JoeCool said:

    The reality is, you come into a job, the product has thousands of warnings, you stress that they should be fixed. People agree, it doesn't change. It is always: we will do it later.

    Yes....or you just have sloppy WTF coworkers who don't really give a damn. I try to leave everything I touch without warnings. It's a losing battle except for those things that no one else touches.



  • @boomzilla said:

    @JoeCool said:
    The reality is, you come into a job, the product has thousands of warnings, you stress that they should be fixed. People agree, it doesn't change. It is always: we will do it later.

    Yes....or you just have sloppy WTF coworkers who don't really give a damn. I try to leave everything I touch without warnings. It's a losing battle except for those things that no one else touches.

    Exactly. I do the same thing - any file I touch, I fix up the warnings. But the count seems to climb anyway



  • @JoeCool said:

    I never said I thought it was OK, so no need for a fucking sample. The reality is, you come into a job, the product has thousands of warnings, you stress that they should be fixed. People agree, it doesn't change. It is always: we will do it later.

    And there's no time for later, because a mostly-functional product MUST be ready by date X, the requirements drastically change halfway through the project, and only 2/3 of the time necessary to get it done right was allocated in the first place.



  • @Liquid Egg Product said:

    @JoeCool said:
    I never said I thought it was OK, so no need for a fucking sample. The reality is, you come into a job, the product has thousands of warnings, you stress that they should be fixed. People agree, it doesn't change. It is always: we will do it later.

    And there's no time for later, because a mostly-functional product MUST be ready by date X, the requirements drastically change halfway through the project, and only 2/3 of the time necessary to get it done right was allocated in the first place.

    Wow, so other people have this problem... We DaveK to come in and change these cultures with the snap of his fingers


Log in to reply